Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make NPM post-processing agnostic to node_modules location #31519

Closed
wants to merge 3 commits into from
Closed

Make NPM post-processing agnostic to node_modules location #31519

wants to merge 3 commits into from

Commits on Jan 20, 2023

  1. refactor: build: reimplement process_npm_assets as shell script 

    Reimplement `pavelib/assets.py:process_npm_assets` equivalently
as `scripts/assets/copy-node-modules.sh`. For backwards compatibility,
`pavelib/assets.py:process_npm_assets` will still exist, just as
a thin wrapper around the new shell script.

Rationale:

* Other parts of pavelib have already been reimplemented, like Python
  unit tests. We're following that trend.
* The Python logic in pavelib is harder to understand than simple
  shell scripts.
* pavelib has dependencies (Python, paver, edx-platform, other libs)
  which means that any pavelib scripts must be executed later in
  the edx-platform build process than we might want them to. For
  example, in a Dockerfile, it might be more performant to process
  npm assets *before* installing Python, but as long as we are still
  using pavelib, that is not an option.
* The benefits of paver have been eclipsed by other tools, like
  Docker (for requisite management) and Click (for CLI building).
* In the next couple commits, we make improvements to
  process-npm-assets.sh. These improvements would have been possible
  in the pavelib implementation, but would have been more complicated.

Part of openedx-unsupported/wg-developer-experience#150
Closes #31606
    @kdmccormick
    kdmccormick committed Jan 20, 2023
    Configuration menu
    Copy the full SHA
    2a2ee88 View commit details
    Browse the repository at this point in the history

  2. build: make node_modules path configurable in copy-node-modules.sh 

    The problem:
  In Tutor, NPM packages are installet d into the openedx image at
  /openedx/edx-platform/node_modules. So, when you bind-mount
  your own edx-platform, you override that node_modules folder.
  Then, you must re-install node_modules yourself
  (`tutor dev run --mount=edx-platform lms npm install`),
  which takes a long time and is completely redundant with the
  node_modules that you had to download when you downloaded
  the openedx image. If you forget to do this, then your
  edx-platform frontend will be broken.

The solution:
  In Tutor: Move node_modules somewhere else in the openedx image, such
  as /openedx/node_modules.

What this commit changes:
  Add a `--node-modules` parameter to copy-node-modules.sh, thus
  helping remove the assumption that node_modules must be a child
  directory of edx-platform.

Part of openedx-unsupported/wg-developer-experience#150
    @kdmccormick
    kdmccormick committed Jan 20, 2023
    Configuration menu
    Copy the full SHA
    1c65ca1 View commit details
    Browse the repository at this point in the history

  3. build: copy edx-ui-toolkit instead of symlinking to it 

    The problem: (See previous commit)
The solution: (See previous commit)

What this commit changes:
  In order to allow certain especially old parts of edx-platform
  to use edx-ui-toolkit without involving webpack, the platform
  has a symlink:
    $EDX_PLATFORM/common/static/edx-ui-toolkit/js ->
    $EDX_PLATFORM/node_modules
  Unfortunately, this symlink is based on the assumption that
  node_modules will always be a child directory of edx-platform.
  In order to eliminate this assumption, we delete the symlink,
  and then have the existing `scripts/assets/copy-node-modules.sh`
  script copy edx-ui-toolkit/js from node_modules over to where the symlink
  used to be.

Part of openedx-unsupported/wg-developer-experience#150
    @kdmccormick
    kdmccormick committed Jan 20, 2023
    Configuration menu
    Copy the full SHA
    1cc3fe3 View commit details
    Browse the repository at this point in the history