Skip to content

Commit 90d8f7c

Browse files
authored
OBS-385: Introduce ESBuild for CSS and Images (#6864)
* OBS-385: use ESBuild to bundle crashstats_base as first test case * OBS-385: remove unwanted npm scripts * OBS-385: create anonymous volumes for node_modules fix lint * OBS-385: add `webapp/static` anonymous volume OBS-385: fix tests * OBS-385: ignore collectstatic css and other things * OBS-385: update ESBuild entrypoints * OBS-385: use staticfiles backend storage instead of pipeline * OBS-385: handle media assets with ESBuild * OBS-385: build CSS and images using ESBuild * OBS-385: update HTML files to use ESBuild CSS and images * OBS-385: revert work-around for status.css * OBS-385: revert staticfiles storage to fix js files not served in production * OBS-385: update collectstatic "ignore" specificity to allow admin css * OBS-385: use npm script in test.sh for consolidation * OBS-397: add ESBuild "watch" mode * OBS-385: remove STATIC_ROOT declaration from local_dev.env * OBS-385: restore bundle quality check, removing PIPELINE_CSS * OBS-385: update webapp documentation
1 parent b541861 commit 90d8f7c

File tree

49 files changed

+1320
-2117
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1320
-2117
lines changed

.devcontainer/Dockerfile

+6-6
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ RUN pip install --no-cache-dir --no-deps -r requirements.txt && \
3636
ENV PYTHONUNBUFFERED=1 \
3737
PYTHONDONTWRITEBYTECODE=1 \
3838
PYTHONPATH=/app \
39-
UGLIFYJS_BINARY=/webapp-frontend-deps/node_modules/.bin/uglifyjs \
40-
CSSMIN_BINARY=/webapp-frontend-deps/node_modules/.bin/cssmin \
41-
NPM_ROOT_PATH=/webapp-frontend-deps/ \
42-
NODE_PATH=/webapp-frontend-deps/node_modules/
39+
UGLIFYJS_BINARY=/app/webapp/node_modules/.bin/uglifyjs \
40+
NPM_ROOT_PATH=/app/webapp/ \
41+
NODE_PATH=/app/webapp/node_modules/
4342

4443
# Install frontend JS deps
45-
COPY --chown=app:app ./webapp/package*.json /webapp-frontend-deps/
46-
RUN cd /webapp-frontend-deps/ && npm install
44+
COPY --chown=app:app ./webapp/package*.json /app/webapp/
45+
RUN cd /app/webapp/ && npm ci
46+

.dockerignore

+2-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,5 @@ build/
44
**/*.pyc
55
__pycache__
66
.cache
7-
webapp-django/node_modules
8-
webapp-django/static
9-
symbols/
7+
node_modules
8+
symbols/

.eslintrc.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
{
22
"env": {
33
"browser": true,
4-
"jquery": true
4+
"jquery": true,
5+
"node": true
56
},
67
"extends": ["plugin:prettier/recommended", "eslint:recommended"],
78
"parserOptions": {

bin/lint.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,5 @@ else
4646

4747
echo ">>> eslint (js)"
4848
cd /app/webapp
49-
/webapp-frontend-deps/node_modules/.bin/eslint .
50-
fi
49+
/app/webapp/node_modules/.bin/eslint .
50+
fi

bin/run_service_webapp.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ if [ "${1:-}" == "--dev" ]; then
2929
echo "Running webapp in local dev environment."
3030
echo "Connect with your browser using: http://localhost:8000/ "
3131
echo "******************************************************************"
32-
cd /app/webapp/ && exec ${CMDPREFIX} python manage.py runserver 0.0.0.0:8000
32+
cd /app/webapp/ && (node esbuild --watch & exec ${CMDPREFIX} python manage.py runserver 0.0.0.0:8000)
3333

3434
else
3535
exec ${CMDPREFIX} gunicorn \

bin/test.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,6 @@ echo ">>> run tests"
5050

5151
# Collect static and then run pytest in the webapp
5252
pushd webapp
53-
${PYTHON} manage.py collectstatic --noinput
53+
npm run build
5454
"${PYTEST}"
5555
popd

docker-compose.override.yml

+7
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,14 @@ services:
44
app:
55
volumes:
66
- .:/app
7+
- /app/webapp/node_modules
8+
- /app/webapp/static
9+
710
test:
811
volumes:
912
- .:/app
13+
- /app/webapp/node_modules
14+
- /app/webapp/static
1015

1116
processor:
1217
volumes:
@@ -19,6 +24,8 @@ services:
1924
webapp:
2025
volumes:
2126
- .:/app
27+
- /app/webapp/node_modules
28+
- /app/webapp/static
2229

2330
stage_submitter:
2431
volumes:

docker/Dockerfile

+8-10
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,23 @@ RUN pip install --no-cache-dir --no-deps -r requirements.txt && \
3333
ENV PYTHONUNBUFFERED=1 \
3434
PYTHONDONTWRITEBYTECODE=1 \
3535
PYTHONPATH=/app \
36-
UGLIFYJS_BINARY=/webapp-frontend-deps/node_modules/.bin/uglifyjs \
37-
CSSMIN_BINARY=/webapp-frontend-deps/node_modules/.bin/cssmin \
38-
NPM_ROOT_PATH=/webapp-frontend-deps/ \
39-
NODE_PATH=/webapp-frontend-deps/node_modules/
36+
UGLIFYJS_BINARY=/app/webapp/node_modules/.bin/uglifyjs \
37+
NPM_ROOT_PATH=/app/webapp/ \
38+
NODE_PATH=/app/webapp/node_modules/
4039

4140
# Install frontend JS deps
42-
COPY --chown=app:app ./webapp/package*.json /webapp-frontend-deps/
43-
RUN cd /webapp-frontend-deps/ && npm install
41+
COPY --chown=app:app ./webapp/package*.json /app/webapp/
42+
RUN cd /app/webapp/ && npm ci
4443

4544
# app should own everything under /app in the container
4645
USER app
4746

4847
# Copy everything over
4948
COPY --chown=app:app . /app/
5049

51-
# Run collectstatic in container which puts files in the default place for
52-
# static files
53-
RUN cd /app/webapp/ && TOOL_ENV=True python manage.py collectstatic --noinput
50+
# Build front-end static files. Runs ESBuild and collectstatic
51+
RUN cd /app/webapp/ && npm run build
5452

5553
# Set entrypoint for this image. The entrypoint script takes a service
5654
# to run as the first argument. See the script for available arguments.
57-
ENTRYPOINT ["/app/bin/entrypoint.sh"]
55+
ENTRYPOINT ["/app/bin/entrypoint.sh"]

docker/config/local_dev.env

-10
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,6 @@ VERSIONS_COUNT_THRESHOLD=0
6262
# Django DEBUG mode which shows settings and tracebacks on errors
6363
DEBUG=True
6464

65-
# Static files are generated as part of the image and reside in
66-
# /app/webapp/static/ which is the default location. Thus for server
67-
# environments, you can leave STATIC_ROOT unset.
68-
#
69-
# For local development, the local directory is mounted as /app so the static
70-
# files generated in the image are not available. For local development,
71-
# static files for the webapp get put in /tmp/crashstats-static/ so we
72-
# need to set STATIC_ROOT to that.
73-
STATIC_ROOT=/tmp/crashstats-static/
74-
7565
# For webapp sessions in the local dev environment, we need to allow cookies to
7666
# be sent insecurely since it's using HTTP and not HTTPS.
7767
SESSION_COOKIE_SECURE=False

docs/service/webapp.rst

+63-92
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,79 @@ Code is in ``webapp/``.
88

99
Run script is ``/app/bin/run_service_webapp.sh``.
1010

11+
Running in a local dev environment
12+
==================================
13+
14+
This documentation assumes you've gone through the setup steps described in the Development chapter :ref:`setup-quickstart`, in particular:
15+
16+
.. code-block:: shell
17+
18+
$ just build
19+
$ just setup
20+
21+
To run the webapp...
22+
23+
.. code-block:: shell
24+
25+
$ docker compose up webapp
26+
27+
...or if you don't like typing:
28+
29+
.. code-block:: shell
30+
31+
$ just run
1132
12-
Configuration
33+
To ease debugging, you can run a shell in the container:
34+
35+
.. code-block:: shell
36+
$ docker compose run --service-ports webapp shell
37+
38+
Then you can start and stop the webapp, adjust files, and debug.
39+
The webapp runs ESBuild's watch mode and Django's StatReloader to reload static file changes automatically.
40+
This avoids needing to stop, rebuild, and restart the container/server on every change.
41+
42+
Static Assets
1343
=============
1444

15-
FIXME
45+
At the time of this writing, JS files are collected and processed by collectstatic and django-pipeline. All other static assets (CSS, images, fonts, etc) are collected and processed by ESBuild.
46+
Migration of JS to ESBuild is currently in progress, with the intent to retire django-pipeline when complete. The collectstatic package will continue to be used in support of the internal Django admin pages.
1647

48+
Static asset builds are triggered by NPM scripts in ``webapp/package.json``. The assets are built into ``/app/webapp/static`` also known as ``STATIC_ROOT``.
1749

18-
Running in a local dev environment
19-
==================================
50+
Production-style Assets
51+
=======================
2052

21-
To run the webapp, do::
53+
When you run ``docker compose up webapp`` in the local development environment,
54+
it starts the web app using Django's ``runserver`` command. ``DEBUG=True`` is
55+
set in the ``docker/config/local_dev.env`` file, so static assets are
56+
automatically served from within the individual Django apps rather than serving
57+
the minified and concatenated static assets you'd get in a production-like
58+
environment.
2259

23-
$ docker compose up webapp
60+
If you want to run the web app in a more "prod-like manner", you want to run the
61+
webapp using ``gunicorn`` and with ``DEBUG=False``. Here's how you do that.
2462

25-
To ease debugging, you can run a shell in the container::
63+
First start a ``bash`` shell with service ports::
2664

2765
$ docker compose run --service-ports webapp shell
2866

29-
Then you can start and stop the webapp, adjust files, and debug.
67+
Compile the static assets (if needed)::
68+
69+
app@socorro:/app$ npm run build --prefix webapp
70+
71+
Then run the webapp with ``gunicorn`` and ``DEBUG=False``::
72+
73+
app@socorro:/app$ DEBUG=False bash bin/run_service_webapp.sh
74+
75+
You will now be able to open ``http://localhost:8000`` on the host and if you
76+
view the source you see that the minified and concatenated static assets are
77+
served instead.
78+
79+
Because static assets are compiled, if you change JS or CSS files, you'll need
80+
to re-run ``npm run build --prefix webapp`` - the "watch mode" feature is not enabled in production.
81+
82+
Admin Account
83+
=============
3084

3185
If you want to do anything in the webapp admin, you'll need to create a
3286
superuser in the Crash Stats webapp and a OIDC account to authenticate against
@@ -80,87 +134,4 @@ A logged-in user can view their detailed permissions on the
80134

81135
The groups and their permissions are defined in
82136
``webapp/crashstats/crashstats/signals.py``. These are applied to
83-
the database in a "post-migrate" signal handler.
84-
85-
86-
Static Assets
87-
=============
88-
89-
In the development environment, the ``STATIC_ROOT`` is set to
90-
``/tmp/crashstats-static/`` rather than ``/app/webapp/static``.
91-
The process in the container creates files with the uid 10001, and Linux users
92-
will have permissions-related problems if these are mounted on the host
93-
computer.
94-
95-
The problem this creates is that ``/tmp/crashstats-static/`` is ephemeral
96-
and any changes there disappear when you stop the container.
97-
98-
If you are on macOS or Windows, then Docker uses a shared file system that
99-
creates files with your user ID. This makes it safe to persist static assets,
100-
at the cost of slower file system performance. Linux users can manually set
101-
the uid and gid to match their account, for the same effect. See "Set UID and
102-
GID for Docker container user" in :ref:`setup-quickstart`.
103-
104-
If you want static assets to persist between container restarts, then you
105-
can override ``STATIC_ROOT`` in ``my.env`` to return it to the ``app`` folder::
106-
107-
STATIC_ROOT=/app/static
108-
109-
Alternatively, you can mount ``/tmp/crashstats-static/`` using ``volumes``
110-
in a ``docker compose.override.yml`` file:
111-
112-
.. code-block:: yaml
113-
114-
version: "2"
115-
services:
116-
webapp:
117-
volumes:
118-
# Persist the static files folder
119-
- ./static:/tmp/crashstats-static
120-
121-
122-
Production-style Assets
123-
=======================
124-
125-
When you run ``docker compose up webapp`` in the local development environment,
126-
it starts the web app using Django's ``runserver`` command. ``DEBUG=True`` is
127-
set in the ``docker/config/local_dev.env`` file, so static assets are
128-
automatically served from within the individual Django apps rather than serving
129-
the minified and concatenated static assets you'd get in a production-like
130-
environment.
131-
132-
If you want to run the web app in a more "prod-like manner", you want to run the
133-
webapp using ``gunicorn`` and with ``DEBUG=False``. Here's how you do that.
134-
135-
First start a ``bash`` shell with service ports::
136-
137-
$ docker compose run --service-ports webapp shell
138-
139-
Then compile the static assets::
140-
141-
app@socorro:/app$ cd webapp/
142-
app@socorro:/app/webapp$ ./manage.py collectstatic --noinput
143-
app@socorro:/app/webapp$ cd ..
144-
145-
Now run the webapp with ``gunicorn`` and ``DEBUG=False``::
146-
147-
app@socorro:/app$ DEBUG=False bash bin/run_service_webapp.sh
148-
149-
You will now be able to open ``http://localhost:8000`` on the host and if you
150-
view the source you see that the minified and concatenated static assets are
151-
served instead.
152-
153-
Because static assets are compiled, if you change JS or CSS files, you'll need
154-
to re-run ``./manage.py collectstatic``.
155-
156-
157-
Running in a server environment
158-
===============================
159-
160-
Add configuration to ``webapp.env`` file.
161-
162-
Run the docker image using the ``webapp`` command. Something like this::
163-
164-
docker run \
165-
--env-file=webapp.env \
166-
mozilla/socorro_app webapp
137+
the database in a "post-migrate" signal handler.

webapp/crashstats/api/jinja2/api/documentation.html

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
{% block site_css %}
44
{{ super() }}
5-
{% stylesheet 'api_documentation' %}
5+
<link rel="stylesheet" href="/static/api/css/documentation.min.css">
66
{% endblock %}
77

88
{% block site_js %}
@@ -111,7 +111,7 @@ <h2><a href="#{{ endpoint.name }}">{{ endpoint.name }}</a></h2>
111111
<div class="run-test">
112112
{% if endpoint.test_drive %}
113113
<button type="submit">Run Test Drive!</button>
114-
<img src="{{ static('img/ajax-loader16x16.gif') }}" alt="Loading..." class="loading-ajax">
114+
<img src="/static/img/ajax-loader16x16.gif" alt="Loading..." class="loading-ajax">
115115
<button type="button" class="close">&times; Close</button>
116116
{% else %}
117117
<em>Test drive not supported.</em>

webapp/crashstats/crashstats/finders.py

+8-9
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@ def find(self, path, all=False):
2020
# staticfiles finders. Before we raise an error, try to find out where,
2121
# in the bundles, this was defined. This will make it easier to correct
2222
# the mistake.
23-
for config_name in "STYLESHEETS", "JAVASCRIPT":
24-
config = settings.PIPELINE[config_name]
25-
for key, directive in config.items():
26-
if path in directive["source_filenames"]:
27-
raise ImproperlyConfigured(
28-
"Static file {} can not be found anywhere. Defined in "
29-
"PIPELINE[{!r}][{!r}]['source_filenames']".format(
30-
path, config_name, key
31-
)
23+
config = settings.PIPELINE["JAVASCRIPT"]
24+
for key, directive in config.items():
25+
if path in directive["source_filenames"]:
26+
raise ImproperlyConfigured(
27+
"Static file {} can not be found anywhere. Defined in "
28+
"PIPELINE[{!r}][{!r}]['source_filenames']".format(
29+
path, "JAVASCRIPT", key
3230
)
31+
)
3332
# If the file can't be found AND it's not in bundles, there's
3433
# got to be something else really wrong.
3534
raise NotImplementedError(path)

webapp/crashstats/crashstats/jinja2/crashstats/product_home.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
{% block site_css %}
44
{{ super() }}
5-
{% stylesheet 'product_home' %}
5+
<link rel="stylesheet" href="/static/crashstats/css/pages/product_home.min.css">
66
{% endblock %}
77

88
{% block page_title %}

0 commit comments

Comments
 (0)