OBS-385: Introduce ESBuild for CSS and Images

[toufali]

Using a modern build tool such as ESBuild allows us to use best-practice CSS features such as @import and advanced optimizations such as bundling, transpilation, and minification without the need for additional 3rd party tools and complex configuration.

This PR is specifically limited to CSS and related image assets, with the goal of introducing ESBuild working in tandem with collectstatic and django-pipeline, which both still process JS files. A PR for using ESBuild on JS files and the complete removal of django-pipeline is forthcoming. (Collectstatic will still remain, however its only concern will be the internal Django admin site.)

Follow-up PRs needed:

• Add "watch" feature to avoid tearing down, rebuilding, and restarting container/server on every CSS/JS change: OBS-397: ESBuild watch mode #6913 Feature is now included in this PR.
• Fix host machine permissions for /app/webapp/static/ and /app/webapp/node_modules/, in particular for Linux users
• Handle JS file builds with ESBuild, removing django-pipeline altogether

To test this PR:
Run the app in both development and production modes following this test plan: https://mozilla-hub.atlassian.net/wiki/spaces/CS1/pages/605028396/Socorro+manual+test+plan#Webapp

1. just build and look out for an ESBuild summary that ends similar to "⚡ Done in 218ms". The collectstatic summary should follow, similar to "328 static files copied to '/app/webapp/static', 4 unmodified, 23 post-processed."
2. just setup
3. just run
4. You should now be running in development. Follow test plan to ensure app functions as expected.
5. To test production, ctrl-c to stop the server.
6. docker compose run --service-ports webapp shell
7. npm run build --prefix webapp (this step – and the similar collectstatic step in the docs might not be needed if files haven't changed since the last build?)
8. DEBUG=False bash bin/run_service_webapp.sh
9. You should now be running in production. Follow test plan to ensure app functions as expected.

[toufali]

Note: the two --ignore flags also ignore css and images for Django admin. From my research, the glob-style flags are notoriously brittle/unintuitive – I was unable to set up crashstats/**/*.css and crashstats/**/img/. Once JS files are included in the new build process, we can more easily do --ignore crashstats so that collectstatic's only concern is admin.

We should remove both ignore flags before merge, but I left them here and in test.sh to illustrate/confirm that ESBuild is in fact building/responsible for CSS/img files.

[toufali]

See note below for webapp/package.json

[willkg]

I skimmed through this and as far as I can tell, it needs some additional changes to work the way we talked about.

First, I think some of the static assets aren't in the right place. In the local dev environment, Django runs with DEBUG=True using the runserver management command, In that mode, it serves assets from the Django app static folders. That mode works with these changes.

However, in our stage and prod environments, Django runs with DEBUG=False using gunicorn and static assets are served from the static/ folder. If we follow the directions here:

https://socorro.readthedocs.io/en/latest/service/webapp.html#production-style-assets

Then we can simulate the stage and prod environments. When we do that, some static assets aren't getting served. I'm seeing HTTP 404s for JS and SVG files for the pages I looked at. I couldn't get very far since JS files weren't getting served and that's affects a large chunk of the navigation.

Second, this doesn't implement something that watches CSS and JS files for changes and rebuilds static assets. In main tip now, you can run the web server and edit JS and CSS files and they get rebuilt and served automatically. With this PR, every time you make a change to JS and CSS files, you have to rebuild the Docker image, set everything back up, and start the webapp container. That's a big step backwards for developer ergonomics. There's nothing in the PR description or followup comments about whether we planned to fix that in a followup PR.

The other thing this needs is a followup PR to fix how /app/webapp/static/ and /app/webapp/node_modules/ get created on the host machine so they have the right permissions. I offered to do that change since it largely affects Linux users, but I wanted to note that it needs to be done and that's not captured anywhere.

In one of our talks, we discussed having esbuild use a different directory tree than the existing webapp/crashstats/APP/static/ for source files that it managed. That alleviates the problems you were having that caused you to add --ignore to collectstatic. Did that solution not work? It's cleaner and clearer than the one implemented in this PR and we know it'll work correctly with Django apps like the Django admin.

This is a complicated enough change that it's worth formalizing a test plan for it and making sure we go through the test steps as we make changes and review. There's no test plan in the PR description or the project proposal. We should formalize one and include it in the description.

[toufali]

@willkg – thanks for taking a look!

However, in our stage and prod environments ... some static assets aren't getting served. I'm seeing HTTP 404s for JS and SVG files for the pages I looked at.

I think this issue was twofold:

• I jumped the gun in replacing part of django-pipeline, forgetting it's still processing JS with this PR. Should be fixed via d39b1de
• I didn't provide instruction on how to manually build for production, and so your test was missing a build step for images. I'll add to the description.

Second, this doesn't implement something that watches CSS and JS files for changes and rebuilds static assets.

From our earlier discussion, I planned to add "watch" via separate PR to keep this one's size/scope down. If you think those changes are better suited here I can add! Else I will update the description to call out this follow-up.

The other thing this needs is a followup PR to fix how /app/webapp/static/ and /app/webapp/node_modules/ get created on the host machine so they have the right permissions.

I will capture this in the description, thanks for the reminder.

In one of our talks, we discussed having esbuild use a different directory tree than the existing webapp/crashstats/APP/static/ for source files that it managed. That alleviates the problems you were having that caused you to add --ignore to collectstatic. Did that solution not work? It's cleaner and clearer than the one implemented in this PR and we know it'll work correctly with Django apps like the Django admin.

I updated the collectstatic --ignore flag to be more specific and not omit the admin folder. It's a very long line unfortunately. This is temporary until we add ESBuild support for JS – the flags will become shorter, though still unsightly in my opinion: -i crashstats -i api ... instead of -i crashstats/*.css -i api/*.css ... It looks like there's a more elegant way to accomplish this, but I didn't have time to figure this out with my limited Django/Python knowledge! I'm not sure how using a different directory for ESBuild helps here? I'm trying to avoid collectstatic needlessly processing files that ESBuild has already done. Would love to chat more about this in case I'm missing something.

This is a complicated enough change that it's worth formalizing a test plan for it and making sure we go through the test steps as we make changes and review. There's no test plan in the PR description or the project proposal. We should formalize one and include it in the description.

💯 I will add this now.

[willkg]

The test plan was very helpful. It should be documented as part of the project--not just in this PR, so we should add it to the proposal and reference that in the relevant Jira issues.

I was able to get through all the testing. I tested the following function areas in both local dev and prod modes:

• product home page
• top crashers report
• signature report
• report view
• super search
• django admin

I noted some issues that should get fixed. One thing that I found confusing is that this PR is summarized as "replace django-pipeline", but it doesn't actually get that far. We're still using django-pipeline and bundling after these changes. We should fix the summary.

PR #6913 covers adding a watch mode. We should include that in this PR. We're on a tight schedule and those two things need to go all-or-nothing and including that PR in this one will (hopefully) reduce the overall iterations of review-and-fix.

In the PR description, it talks about a follow-up change to rework how JS files are managed. How far along is that work? If we land this, will we need to complete that work, too?

Looking good so far!

[willkg]

We're not using the stylesheet and static template functions anymore which means these are all hard-coded paths to the destination for these files. In order to know the destination, a developer needs to know how the files are transformed from the source.

We should document how all the different files are transformed, by what, and where they end up so this is easier to reason about.

Looks like this PR doesn't update the docs. I think we want to update several sections in this file:

https://github.com/mozilla-services/socorro/blob/main/docs/service/webapp.rst

Particularly the "Static Assets" and "Production-style Assets" sections. Also, the "Running in a server environment" section is probably wrong--we should just remove that.

[willkg]

These scripts should be documented in the webapp service docs.

[toufali]

PR #6913 covers adding a watch mode. We should include that in this PR. We're on a tight schedule and those two things need to go all-or-nothing and including that PR in this one will (hopefully) reduce the overall iterations of review-and-fix.

Watch mode added via 857ab27

[willkg]

This looks good. I went through the test plan and everything looked fine with that. I also verified we can edit static asset source files and they get rebuilt when running the webapp in a local dev environment.

• I tested a CSS file -- added and removed setting background to red; esbuild watch kicked in and updated it
• I tested updating a png file -- copied a different png over it; esbuild watch kicked in and updated it
• I tested a JS file -- added and removed an alert; django-pipeline sent new one in next request

I really like how esbuild watch works--it explicitly states that it saw something change and rebuilds it.

webapp-1      | [watch] build started (change: "crashstats/crashstats/static/crashstats/css/base/layout.css")
webapp-1      | [watch] build finished

That's very nice!

The only thing left here is docs. I'm going to approve this now and whatever docs changes you make we'll go with--no need for another review. We can do a followup fix later for anything we need to change.

Once this deploys to stage, we will need to go through the webapp test plan to make sure it's working correctly in a server environment. It probably will, but we should verify that.