Switch frontend app bundler #457

aaronxsu · 2022-04-21T21:57:26Z

Overview

This PR has made the following changes as a result of switching away from mastarm:

Demo

App served from Django root

App in the map view

Notes

I tried to keep the formatter change and the linter change separate from the meaningful code changes
The following will be in their own issues:

handle env-based dotenv files better, pull dotenv file in bootstrap script
favicons

Testing Instructions

Pull down this branch
At the root of the repo, run: cp ./src/frontend/.env.template .env. Then fill in the fields in .env by the one stored on lastpass
At the root of the repo, run ./scripts/update, which will build the frontend and backend, install frontend deps, create a static build of the frontend to be used by django, and run django manage scripts
AT the root of the repo, run ./scripts/server, which will bring both frontend and backend up and running. Navigate to http://localhost:9966 to access app frontend. If you make changes to frontend code, module hot reloading should work. Navigate to http://localhost:8085 to access the static frontend app served from django.

Resolves #376

docker-compose.yml

src/backend/Dockerfile

src/frontend/.eslintrc.json

src/frontend/package.json

src/frontend/webpack.config.js

src/frontend/package.json

ddohler

This is looking really good! I had a few comments, but the main thing I'm uncertain about is whether neighborhood images are working or not, and how to check. When I view the map page, I get several 404 errors in the console looking for http://localhost:9966/assets/neighborhoods/02203_town_square.jpg in order to display the neighborhood thumbnails.

Under mastarm, the neighborhood thumbnails are downloaded by running ./neighborhood_data/update_data.sh and then (I think?) manually copying the images to the assets folder. We are planning to migrate neighborhood info to the database so that it can be managed via the Django API, so I think we should avoid making major changes to the neighborhood data management process (because they'll just get thrown away in a few sprints), but we should make sure that we don't break the existing neighborhood images.

src/frontend/webpack.config.js

src/frontend/src/index.js

src/frontend/.eslintrc.json

docker-compose.yml

src/backend/Dockerfile

src/frontend/Dockerfile

aaronxsu · 2022-05-05T00:51:38Z

@ddohler good find on the bug that the neighborhood image thumbnails not showing up. I fixed it in this commit: 16dfc1d . It was simply because assets subdirectory do not exist anymore under this new bundler configuration. Such assets should now be in frontend/public.

If you still have those thumbnails lying around in the neighborhood_data directory, please feel free to do something similar to these changes: 16dfc1d#diff-a7a011c1c5fb7bd090210ada6e19390c174069715fab4772fd4e614789a47e26R16 - just copy everything in neighborhood_data/images to src/frontend/public/neighborhoods. Then ./scripts/server should run the frontend with those images. If you don't have the thumbnails anymore, re-running the ./update_data.sh in neighborhood_data directory should do it automatically.

This gets me thinking:

If this step should be added in the cibuild before creating the static frontend bundle
Do these images ever change? Or are they different per environment? If they are really static, would it be easier, for both dev and for deployment, to upload them all to S3, and pull them down before bundling frontend? Since the current workflow seems to be running the update_data script first no matter what.
Could you share a bit more about your vision of having these images in DB? What would the schema be like for such a table? Does it mean all neighborhood JSON data will be in a new table too? Should they be stored as bytes or base64 encoded string? Because I think this might be a better approach than 1 and also 2 above.

aaronxsu · 2022-05-05T00:52:01Z

I am aware of the merge conflicts, and I plan to address them before merging.

ddohler

Looks good to me! I had one further question about how environment variables are being injected, but I think this is ready to go once we've got that sorted out and the merge conflicts are resolved.

Could you share a bit more about your vision of having [neighborhood] images in DB? What would the schema be like for such a table? Does it mean all neighborhood JSON data will be in a new table too? Should they be stored as bytes or base64 encoded string? Because I think this might be a better approach than 1 and also 2 above.

Yes, we're going to migrate all the data that's currently in the neighborhoods.json and neighborhood_bounds.json files into database fields. For the neighborhood images specifically, I'm not exactly sure how we'll handle them -- I think storing them as binary blobs in the database could work since they're relatively small, or maybe it would be better to upload them to S3. But either way, we should allow admins to edit the images from the Django admin page, similar to this: https://github.com/azavea/cac-tripplanner/blob/develop/python/cac_tripplanner/cms/models.py#L96 . That way the admins can change the photos or add new neighborhoods if they need to. How does that sound?

src/frontend/webpack.config.js

scripts/cibuild

aaronxsu · 2022-05-10T02:11:40Z

@ddohler, you are right about how env vars are read in through the way you suggested, I made the change here: 14ca820

Regarding your vision of how neighborhood data will be stored and managed, yes it sounds good and makes sense to me.

Somehow I was seeing issues fetching json from those cloudfront URLs again. Did you see that from your end when doing the re-review?

ddohler · 2022-05-10T14:38:52Z

Somehow I was seeing issues fetching json from those cloudfront URLs again. Did you see that from your end when doing the re-review?

Oh, good catch! I didn't actually check the console when I was reviewing, but I do see those errors now. But I'm also seeing this error and I think it may be the root cause: Uncaught Error: Bounds are not valid.

If the bounds aren't valid then it's similar to if you tried to use an address outside the Boston area, and you might see errors loading the network files. I suspect this indicates that neighborhood_bounds.json may not be being loaded properly. I don't see a request for it on this branch but on develop I do. As discussed in RRP, this seems to be happening across all environments, so it's unlikely to be something specific to this branch.

Update docker-compose: - to have a `app` service that replaces `taui` - to mount the frontend/build to backend/static so that django renders the app at domain:port/ Switch bundler, update dir structure: - remove mastarm - add webpack and babel as dev deps - add configs for the above 2 dev deps - hard-code redux store init vals - read env vars from dotenv - update task runner scripts in package.json - move taui into src and rename to src/frontend - update some imports in the JS code Update webpack config Remove mock Add eslint and its related; Add prettier-eslint Add style lint scripts Update webpack config Run yarn fixlint-js: these are automatic fixes Add prettier. Run format script with prettier Added eslint rules to deal with later Do not check style files in /build dir Update cibuild script Install frontend dep before running tests Add node 12 base image for frontend Update node image Update scripts, docker-compose, and dockerfile Update env var frontend config Update per request Update the way to read in env vars Update how neighborhood thumbnail is downloaded and read Address PR comments: - update how process.env is read - add url to issue for todo in cibuild - remove console.log

* Update bootstrap script to pull down dotenv * Add bootstrap step to CI to load dotenv * Improve logging of steps in cibuild * Add format script for frontend app * Update build context for Django service * Update PR template to suggest running format script Co-authored-by: Derek Dohler <[email protected]>