build: `npm run build` (experimental) #32823

kdmccormick · 2023-07-24T14:26:25Z

This PR implements much of: https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst

This is backwards-compatible. paver update_assets should not be affected.

The new command warns that it is "experimental" for a few reasons:

Currently, npm run build will fail in the webpack phase unless you first run xmodule_assets. This will be changed soon, so I did not feel that it would be worthwhile to add a separate special npm run xmodule-assets step.
I have tested the new build, but not quite so thoroughly that I'd recommend it as the production default yet. In fact, once the xmodule_assets work lands, I'd like to share this on the forums so early adopters can try it out. That'll make me more comfortable making it the default over Paver.
The commands lack some top-level documentation. Once they stabilize more, I'd like to add a section to the README that explains how and when to use npm run build, its subcommands (npm run webpack[-dev], npm run compile-sass[-dev]), and the environemnt variables (STATIC_ROOT_LMS, etc).
npm run watch is not yet implemented. The current asset-watching that edx-platform and Tutor provide both rely on Paver.

Part of: #31604

Testing

There are a few different ways you could test this:

Directly from your host, preview the Sass help & output:

npm run compile-sass -- --help
npm run compile-sass -- --dry --theme-dir=themes`

Using Tutor Nightly dev:

tutor mounts add path/to/edx-platform
tutor dev run lms npm clean-install
tutor dev run lms openedx-assets xmodule
tutor dev run lms npm run build-dev
tutor dev start -d

Using Tutor Nightly prod:

tutor mounts add path/to/edx-platform
tutor local run lms npm clean-install
tutor local run lms openedx-assets xmodule
tutor local run lms npm run build
tutor local start -d
tutor local exec lms ./manage.py lms collectstatic --noinput
tutor local exec cms ./manage.py cms collectstatic --noinput

If you're adventurous: directly from your host, try:

npm clean-install
pip install -r requirements/edx/base.txt  # (would be assets.txt if not for xmodule_assets command)
xmodule_assets common/static/xmodule
npm run build-dev

kdmccormick

Notes for reviewers 👀

kdmccormick · 2023-07-24T15:11:28Z

package.json

+    "postinstall": "scripts/copy-node-modules.sh",
+    "build": "echo 'WARNING: `npm run build` in edx-platform is experimenal. Use at your own risk.' && npm run webpack && npm run compile-sass",
+    "build-dev": "echo 'WARNING: `npm run build-dev` in edx-platform is experimenal. Use at your own risk.' && npm run webpack-dev && npm run compile-sass-dev",
+    "webpack": "NODE_ENV=${NODE_ENV:-production} \"$(npm bin)/webpack\" --progress --config=${WEBPACK_CONFIG_PATH:-webpack.prod.config.js}",
+    "webpack-dev": "NODE_ENV=development \"$(npm bin)/webpack\" --progress --config=webpack.dev.config.js",
+    "compile-sass": "scripts/compile_sass.py --env=${NODE_ENV:-production}",
+    "compile-sass-dev": "scripts/compile_sass.py --env=development"


I'd love to add comments here, but I can't since it's JSON. I do plan to document these in edx-platform's README, though, once they stabilize more.

In short:

npm run webpack is a super thin wrapper around webpack.

npm run compile-sass calls a script (currently written in Python, maybe one day written in Bash) which compiles Sass into CSS.

npm run build is a combination of the above.

The -dev targets are just simple variations of the base commands, for the convenience of developres.

Wherever it was possible, I've written these so that:

You can override them with en vars, eg WEBPACK_CONFIG_PATH=my-special-config.js npm run webpack

You can provide extra options: npm run compile-sass -- --theme-dir=/my/cool/themes

kdmccormick · 2023-07-24T15:14:08Z

requirements/edx/paver.in

-libsass==0.10.0                     # Python bindings for the LibSass CSS compiler
+libsass                             # Python bindings for the LibSass CSS compiler


Constraint is moved over to constraints.txt so that it can be shared by assets.txt.

kdmccormick · 2023-07-24T15:15:44Z

scripts/compile_sass.py

+    "theme_dirs",
+    metavar="PATH",
+    multiple=True,
+    envvar="EDX_PLATFORM_THEME_DIRS",


This is a new env var, and is mentioned in the ADR. Eventually, I will document it in the edx-platform README.

kdmccormick · 2023-07-24T15:24:39Z

webpack.common.config.js

+var staticRootLms = process.env.STATIC_ROOT_LMS || "./test_root/staticfiles";
+var staticRootCms = process.env.STATIC_ROOT_CMS || (staticRootLms + "/studio");


The goal of all the changes in this file is to provide some reasonable defaults for all env vars, so that webpack can be run on its own without having to supply a pile of environment variables.

We could have shoved all these defaults into the npm run webpack call in package.json, but that seemed worse than putting them here.

package.json

feanil

I tried to run this after running make clean in edx-platform.

npm install --legacy-peer-deps
npm run build

I got the following error:

Error: Cannot find module './common/static/xmodule/webpack.xmodule.config.js'

Did a file not get checked in or did I miss a step somewhere?

kdmccormick · 2023-07-25T15:52:37Z

@feanil Yep, you missed the xmodule_assets common/static/xmodule, which will be necessary until we merge #32685.

feanil

Ah, doh, missed the bottom of the description. But yes that was it, it works now, I was able to get all the assets built locally without any issue.

See https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst Part of: openedx#31604

edx-pipeline-bot · 2023-07-26T13:13:15Z

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot · 2023-07-26T13:32:37Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2023-07-26T13:32:38Z

2U Release Notice: This PR has been deployed to the edX production environment.

kdmccormick force-pushed the kdmccormick/npm-run-build branch from 006ffe0 to 1175a73 Compare July 24, 2023 15:20

kdmccormick commented Jul 24, 2023

View reviewed changes

kdmccormick marked this pull request as ready for review July 24, 2023 15:25

kdmccormick requested a review from feanil July 24, 2023 15:27

kdmccormick commented Jul 24, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

kdmccormick force-pushed the kdmccormick/npm-run-build branch from 646d452 to 40578a4 Compare July 24, 2023 16:41

This was referenced Jul 24, 2023

build: npm run webpack (WIP) #32721

Closed

build: npm run compile-sass (WIP) #32722

Closed

build: rewrite static asset build #31912

Closed

Build edx-platform assets without Paver #31604

Closed

feanil reviewed Jul 25, 2023

View reviewed changes

feanil approved these changes Jul 25, 2023

View reviewed changes

kdmccormick added 3 commits July 26, 2023 00:13

build: npm run webpack

46e5624

See https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst Part of: openedx#31604

build: npm run compile-sass

468bfae

See https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst Part of: openedx#31604

build: npm run build

e1a88d9

See https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst Part of: openedx#31604

kdmccormick force-pushed the kdmccormick/npm-run-build branch from 40578a4 to e1a88d9 Compare July 26, 2023 04:13

kdmccormick merged commit 3e67719 into openedx:master Jul 26, 2023

kdmccormick deleted the kdmccormick/npm-run-build branch July 26, 2023 12:04

UsamaSadiq mentioned this pull request Jul 27, 2023

fix: include new assets.txt file in make upgrade #32860

Merged

kenclary mentioned this pull request Aug 4, 2023

fix: fix mako runtime bug with studio_view. TNL-10912 #32912

Merged

kdmccormick mentioned this pull request May 1, 2024

[DEPR]: Paver #34467

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: `npm run build` (experimental) #32823

build: `npm run build` (experimental) #32823

kdmccormick commented Jul 24, 2023 •

edited

Loading

kdmccormick left a comment

kdmccormick Jul 24, 2023

kdmccormick Jul 24, 2023

kdmccormick Jul 24, 2023

kdmccormick Jul 24, 2023

feanil left a comment

kdmccormick commented Jul 25, 2023

feanil left a comment

edx-pipeline-bot commented Jul 26, 2023

edx-pipeline-bot commented Jul 26, 2023

edx-pipeline-bot commented Jul 26, 2023

		libsass==0.10.0 # Python bindings for the LibSass CSS compiler
		libsass # Python bindings for the LibSass CSS compiler

		var staticRootLms = process.env.STATIC_ROOT_LMS \|\| "./test_root/staticfiles";
		var staticRootCms = process.env.STATIC_ROOT_CMS \|\| (staticRootLms + "/studio");

build: npm run build (experimental) #32823

build: npm run build (experimental) #32823

Conversation

kdmccormick commented Jul 24, 2023 • edited Loading

Testing

kdmccormick left a comment

Choose a reason for hiding this comment

kdmccormick Jul 24, 2023

Choose a reason for hiding this comment

kdmccormick Jul 24, 2023

Choose a reason for hiding this comment

kdmccormick Jul 24, 2023

Choose a reason for hiding this comment

kdmccormick Jul 24, 2023

Choose a reason for hiding this comment

feanil left a comment

Choose a reason for hiding this comment

kdmccormick commented Jul 25, 2023

feanil left a comment

Choose a reason for hiding this comment

edx-pipeline-bot commented Jul 26, 2023

edx-pipeline-bot commented Jul 26, 2023

edx-pipeline-bot commented Jul 26, 2023

build: `npm run build` (experimental) #32823

build: `npm run build` (experimental) #32823

kdmccormick commented Jul 24, 2023 •

edited

Loading