Skip to content

Commit

Permalink
Fix bug 1473330 - Translate.Next architecture (mozilla#989)
Browse files Browse the repository at this point in the history
* Add React frontend for Translate app.
This has been set up using create-react-app.

* Integrate new Translate app into django.
This provides a working prod and dev environment. In production, django will serve the index.html file as a template, and files built by webpack will be collected and distributed with other django static files. In development, all requests are proxied to the dev webpack server, allowing all dev niceties to be used.

* Add support for websockets.

* Add a bit of documentation specific to our use case.

* Add redux for state management.

* Show a very basic list of entities.

* Enable absolute path import.

* Add links to various resources.

* Rearchitecture code into modules.

All features should be self-contained into a module in src/modules/. All code that is shared amongst several modules should go into a module in src/core/.

* Remove unused assets.

* Add some architecture documentation.

* Add Flow to current code for type checking.

* Add unit testing with jest, enzyme and sinon.

* Extend documentation, talk about type checking, modules, and list tools we use.

* Move CSS rules closer to actual component.

* Add django-waffle and hide translate.next behind a switch.

* Better structure for the README file.

* Build and test frontend in travis.

* Of course it is better if dependencies are installed.

* Use pushd to run commands in frontend.

* Configure eslint and fix errors.

* Add more code comments.

* Improve documentation around local dev and Flow.

* Add tests for the Translate view.

* Improve documentation.

* Fix serving static files for development.

* Integrate Translate.Next with out docker setup.

* Remove debug.

* Use DEV instead of DEBUG.

* Much review.
          Many comments.
    Wow better!

* Flatter module public interfaces.

* Import correctly from modules.

* Fix docker webapp run.
  • Loading branch information
adngdb authored Jul 18, 2018
1 parent 77d1b6d commit 43becde
Show file tree
Hide file tree
Showing 43 changed files with 8,619 additions and 24 deletions.
4 changes: 4 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
**/*.bundle.js
**/*.js.map
**/dist/**
**/build/**
vendor/**
webpack.config.js
coverage/**
Expand All @@ -10,4 +11,7 @@ static/*
**/app/error_pages/**/*.js
**/*blockrain*js
assets/*
**/node_modules/**
docs/

pontoon/base/templates/js/pontoon.js
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
env: {
es6: true,
browser: true,
jest: true,
},
parser: "babel-eslint",
parserOptions: {
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ tmp/*
.vagrant
/env/
/static/
.env
/.env
docs/_build
venv
/media/
Expand Down
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ install:
- nvm install node
- nvm use node
- npm install .
- pushd frontend && npm install && popd
script:
- pylama pontoon
- pylama tests
- ./node_modules/.bin/webpack
- pushd frontend && npm run build && popd
- python manage.py collectstatic -v0 --noinput
- npm test
- pushd frontend && npm test && popd
- py.test --cov-append --cov-report=term --cov=. -v --create-db --migrations
- ./node_modules/.bin/eslint .
- codecov
Expand Down
18 changes: 17 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ clean:
test:
./docker/run_tests_in_docker.sh ${ARGS}

test-frontend:
${DOCKER} run --rm \
-v `pwd`:/app \
--workdir /app/frontend \
--tty \
--interactive \
local/pontoon yarn test

flow:
${DOCKER} run --rm \
-v `pwd`:/app \
-e SHELL=bash \
--workdir /app/frontend \
--tty --interactive \
local/pontoon yarn flow:dev

shell:
./docker/run_tests_in_docker.sh --shell

Expand All @@ -65,7 +81,7 @@ build-frontend-w: assets
-v `pwd`:/app \
--workdir /app \
-e LOCAL_USER_ID=$UID \
--tty \
--tty --interactive \
local/pontoon npm run build-w

# Old targets for backwards compatibility.
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ services:
command: ["/app/docker/run_webapp.sh"]
ports:
- "8000:8000"
- "3000:3000"
volumes:
- .:/app
entrypoint:
Expand Down
56 changes: 38 additions & 18 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,52 @@ RUN useradd --shell /bin/bash -c "" -m app
COPY requirements.txt /app/requirements.txt
COPY requirements-dev.txt /app/requirements-dev.txt
COPY requirements-test.txt /app/requirements-test.txt
RUN pip install -U 'pip>=8'
RUN pip install --no-cache-dir --require-hashes -r requirements-dev.txt
RUN pip install --no-cache-dir --require-hashes -r requirements-test.txt
RUN pip install -U 'pip>=8' && \
pip install --no-cache-dir --require-hashes -r requirements-dev.txt && \
pip install --no-cache-dir --require-hashes -r requirements-test.txt

# Install nodejs and npm from Nodesource's 8.x branch
# RUN DEBIAN_FRONTEND=noninteractive apt-get install -y curl
# Install nodejs and npm from Nodesource's 8.x branch, as well as yarn
RUN curl -s https://deb.nodesource.com/gpgkey/nodesource.gpg.key | apt-key add - && \
echo 'deb https://deb.nodesource.com/node_9.x jessie main' > /etc/apt/sources.list.d/nodesource.list && \
echo 'deb-src https://deb.nodesource.com/node_9.x jessie main' >> /etc/apt/sources.list.d/nodesource.list
RUN DEBIAN_FRONTEND=noninteractive apt-get update
RUN DEBIAN_FRONTEND=noninteractive apt-get install -y nodejs
RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - && \
echo 'deb https://dl.yarnpkg.com/debian/ stable main' > /etc/apt/sources.list.d/yarn.list
RUN DEBIAN_FRONTEND=noninteractive apt-get update && \
DEBIAN_FRONTEND=noninteractive apt-get install -y nodejs yarn

# Create some folders and give them to the app user
RUN mkdir -p /webapp-frontend-deps && chown app:app /webapp-frontend-deps
RUN mkdir -p /webapp-frontend-deps && \
chown app:app /webapp-frontend-deps

RUN chown app:app /app

# Create the folder for front-end assets
RUN mkdir -p /webapp-frontend-deps/assets && chown app:app /webapp-frontend-deps/assets
RUN mkdir -p /webapp-frontend-deps/assets && \
chown app:app /webapp-frontend-deps/assets

# Create the folders for Translate.Next
RUN mkdir -p /app/frontend && \
mkdir -p /webapp-frontend-deps/frontend/build

# Link to the node modules installed with npm. We install those in a different folder because
# when using this container in development, we replace the /app folder with a volume. By putting
# those dependencies outside of /app, we can reuse them more easily.
RUN ln -s /webapp-frontend-deps/node_modules /app/node_modules
RUN ln -s /webapp-frontend-deps/assets /app/assets
RUN chown -R app:app /webapp-frontend-deps

# Install node requirements
COPY ./package.json /webapp-frontend-deps/package.json
COPY ./package-lock.json /webapp-frontend-deps/package-lock.json
RUN cd /webapp-frontend-deps/ && npm install
RUN cd /webapp-frontend-deps && npm install

COPY ./frontend/package.json /webapp-frontend-deps/frontend/package.json
COPY ./frontend/yarn.lock /webapp-frontend-deps/frontend/yarn.lock
RUN cd /webapp-frontend-deps/frontend && yarn install

# Link to the node modules installed with npm and yarn. We install those in
# different folders because when using this container in development,
# we replace the /app folder with a volume. By putting those dependencies
# outside of /app, we can reuse them more easily.
RUN ln -s /webapp-frontend-deps/node_modules /app/node_modules && \
ln -s /webapp-frontend-deps/assets /app/assets && \
ln -s /webapp-frontend-deps/frontend/node_modules /app/frontend/node_modules && \
ln -s /webapp-frontend-deps/frontend/build /app/frontend/build

COPY . /app/
COPY ./docker/config/webapp.env /app/.env
Expand All @@ -55,11 +72,14 @@ ENV PYTHONPATH /app
ENV WEBPACK_BINARY /app/node_modules/.bin/webpack
ENV YUGLIFY_BINARY /app/node_modules/.bin/yuglify

# Run webpack to compile JS files
RUN cd /app/ && $WEBPACK_BINARY

# Build Translate.Next frontend resources
RUN cd /app/frontend/ && yarn build

# Run collectstatic in container which puts files in the default place for
# static files.
RUN cd /app/ && python manage.py collectstatic --noinput

# Run webpack to compile JS files
RUN cd /app/ && $WEBPACK_BINARY

CMD ["/app/docker/run_webapp.sh"]
4 changes: 3 additions & 1 deletion docker/entrypoint_webapp.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#!/bin/bash

# Make sure a link to the node modules is created.
# Make sure links to the node modules are created.
ln -s /webapp-frontend-deps/node_modules /app/node_modules
ln -s /webapp-frontend-deps/frontend/node_modules /app/frontend/node_modules
ln -s /webapp-frontend-deps/frontend/build /app/frontend/build
ln -s /webapp-frontend-deps/assets /app/assets

# Add local user
Expand Down
5 changes: 4 additions & 1 deletion docker/run_webapp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,8 @@ git rev-parse HEAD > static/revision.txt
echo ">>> Setting up the db for Django"
python manage.py migrate

echo ">>> Starting frontend build process in the background"
cd frontend && yarn start &

echo ">>> Starting local server"
python manage.py runserver 0.0.0.0:8000
python manage.py runserver --nostatic 0.0.0.0:8000
1 change: 1 addition & 0 deletions frontend/.env
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
NODE_PATH = 'src/'
14 changes: 14 additions & 0 deletions frontend/.flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[ignore]

[include]

[libs]

[lints]

[options]
module.system=node
module.system.node.resolve_dirname=node_modules
module.system.node.resolve_dirname=./src

[strict]
21 changes: 21 additions & 0 deletions frontend/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# See https://help.github.com/ignore-files/ for more about ignoring files.

# dependencies
/node_modules

# testing
/coverage

# production
/build

# misc
.DS_Store
.env.local
.env.development.local
.env.test.local
.env.production.local

npm-debug.log*
yarn-debug.log*
yarn-error.log*
162 changes: 162 additions & 0 deletions frontend/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
# Translate.Next — a better Translate app for Pontoon


## Tools

<table>
<tr>
<td>Bootstrapper</td>
<td>create-react-app — https://github.com/facebook/create-react-app</td>
</tr>
<tr>
<td>Package manager</td>
<td>Yarn — https://yarnpkg.com/</td>
</tr>
<tr>
<td>Views</td>
<td>React — https://reactjs.org/</td>
</tr>
<tr>
<td>Data</td>
<td>Redux — https://redux.js.org/</td>
</tr>
<tr>
<td>Tests</td>
<td>Jest — http://jestjs.io/</td>
</tr>
<tr>
<td>Type checking</td>
<td>Flow — https://flow.org/</td>
</tr>
<tr>
<td>Localization</td>
<td>Fluent — https://projectfluent.org/</td>
</tr>
<tr>
<td>Style</td>
<td>CSS</td>
</tr>
</table>


## Code architecture

### Where code goes

`src/core/` contains features that are shared in the application, in the form of modules. There should be as little code as possible in this folder.

`src/modules/` contains the self-contained features of the application, separated in modules.

`src/rootReducer.js` creates the main reducer to be used with Redux. When adding a new module with a reducer, make sure to include that reducer to `rootReducer`.

### Modules

Each module should be in a folder and have an `index.js` file that serves as the module's public interface. Outside of the module, always import from the module's index, and never from specific files. This allows us to easily evolve modules and keep things decoupled, which reduces code complexity.

Here are the files commonly found in a module:

- `index.js` — public interface of the module
- `actions.js` — actions that can be used to fetch data from an API, trigger changes, etc.
- `reducer.js` — a single Redux reducer (if you need to have more than one reducer, you probably actually need to make several modules)
- `constants.js` — a list of constants required by the module or other modules. It is recommended to define a NAME constant here which should be a unique identifier of the module. This name will be used in `combineReducers` and in all `mapStateToProps` to identify the subtree of the global state relevant to this module.
- `components/` — a folder containing components, with file names in CamelCase

Of course, more can be added if needed. For example, modules with a high number of action types might want to have an `actionTypes.js` file to separate them from actions.


## Running and deploying

### This feature is behind a Switch

While this is under development, the feature is hidden behing a feature switch, and thus is not accessible by default. In order to turn it on, you have to run `./manage.py waffle_switch translate_next on --create`, then restart your web server. To turn it off, run `./manage.py waffle_switch translate_next off`.

### Production

The only required step for the front-end is to build static files with `yarn build`. Django is configured to collect the `index.html` and static files from the `build` folder and put them with other static files. All of that is automated for deployement to Heroku.

### Development

If you're using docker, `make run` automatically starts both a webpack server (on port 3000) and a Django server (on port 8000). Django is the server you want to hit, and it will then proxy appropriate requests to the webpack server.

A common case during development is to have 3 terminals open: one for the dev servers, one for the tests and one for Flow:

# terminal 1
$ make run

# terminal 2
$ make test-frontend

# terminal 3
$ make flow

#### Enabling websocket and warm-reloading for dev

Currently websocket requests are redirected by Django to the webpack server. Sadly, by default major browsers do not support websocket redirection. To enable it in Firefox, go to `about:config` and turn `network.websocket.auto-follow-http-redirect` to `true`. Note that there is a bug filed to get rid of that option entirely: https://bugzilla.mozilla.org/show_bug.cgi?id=1052909

As far as we know, it is not possible to make that work in Chrome or Edge. This only impacts development, as there's no hot reloading in production.

If you can't turn on websockets, you will see errors in the console (that's not very impacting) and you'll have to reload your Django server regularly, because polling requests don't close, and after so many web page reloads, the Django process won't be able to accept new requests.


## Type checking

Our code uses Flow to enforce type checking. This is a good way to significantly improve resilience to bugs, and it removes some burden from unit tests (because Flow ensures that we use functions and components correctly).

To check for Flow issues during development while you edit files, run:

yarn flow:dev

To learn more, you can read [Why use static types in JavaScript?](https://medium.freecodecamp.org/why-use-static-types-in-javascript-part-1-8382da1e0adb) or the official [Flow documentation](https://flow.org/en/docs/). Additionally, you can read through the [web-ext guide](https://github.com/mozilla/web-ext/blob/master/CONTRIBUTING.md#check-for-flow-errors) for hints on how to solve common Flow errors.

Until we define our set of rules, please refer to the [addons team's Flow manifesto](https://github.com/mozilla/addons-frontend/#flow) regarding specific usage and edge cases.


## Testing

Tests are run using [`jest`](https://facebook.github.io/jest/). We use [`enzyme`](http://airbnb.io/enzyme/docs/api/) for mounting React components and [`sinon`](http://sinonjs.org/) for mocking.

To run the test suite, use:

$ yarn test

It will start an auto-reloading test runner, that will refresh every time you make a change to the code or tests.

Tests are put in files called `fileToTest.test.js` in the same directory as the file to test. Inside test files, test suites are created. There should be one test suite per component, using this notation:

```javascript
describe('<Component>', () => {
// test suite here
});
```

Individual tests follow `mocha`'s syntax:

```javascript
it('does something', () => {
// unit test here
});
```

We use `jest`'s [`expect`](https://facebook.github.io/jest/docs/en/expect.html) assertion tool.


## Development resources

### Integration between Django and React

- [Making React and Django play well together — Fractal Ideas](https://fractalideas.com/blog/making-react-and-django-play-well-together/)
- [Making React and Django play well together - the “hybrid app” model — Fractal Ideas](https://fractalideas.com/blog/making-react-and-django-play-well-together-hybrid-app-model/)

### Code architecture

- [Three Rules For Structuring (Redux) Applications — Jack Hsu](https://jaysoo.ca/2016/02/28/organizing-redux-application/)
- [The Anatomy Of A React & Redux Module (Applying The Three Rules) — Jack Hsu](https://jaysoo.ca/2016/02/28/applying-code-organization-rules-to-concrete-redux-code/)
- [Additional Guidelines For (Redux) Project Structure — Jack Hsu](https://jaysoo.ca/2016/12/12/additional-guidelines-for-project-structure/)

### Tools documentation

- [React](https://reactjs.org/docs/getting-started.html)
- [Redux](https://redux.js.org/)
- [create-react-app](https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md)
- [Flow](https://flow.org/en/docs/)
- [Jest](http://jestjs.io/docs/en/getting-started)
Loading

0 comments on commit 43becde

Please sign in to comment.