Skip to content

Commit

Permalink
perf: move node_modules out of edx-platform repo
Browse files Browse the repository at this point in the history
Background: NPM packages were installed into the openedx image at
/openedx/edx-platform/node_modules. So, when one mounted their own copy of
edx-platform, the built-in node_modules folder was overriden. This
forced edx-platform developers to re-run ``npm install``, which is
redundant, slow, resource-intensive, and added complexity to the first-time
developer setup process. If one forgot to run ``npm install``, their
LMS/CMS frontend would be broken. And, when edx-platform's NPM requirements
were updated, developers would not receive these updates from the openedx
image; they would need to think to re-run ``npm install`` themselves.

The solution: Move /openedx/edx-platform/node_modules to
/openedx/node_modules. Note that this new location is outside of the
edx-platform repository. So, when a developer mounts their own copy of
edx-platform, the image's node_modules will remain intact.

Closes openedx-unsupported/wg-developer-experience#150
  • Loading branch information
kdmccormick committed Jan 18, 2023
1 parent 0f67506 commit 45e9e40
Show file tree
Hide file tree
Showing 5 changed files with 313 additions and 30 deletions.
10 changes: 10 additions & 0 deletions changelog.d/20230110_165850_kdmc_global_node_modules.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!--
Create a changelog entry for every new user-facing change. Please respect the following instructions:
- Indicate breaking changes by prepending an explosion 💥 character.
- Prefix your changes with either [Bugfix], [Improvement], [Feature], [Security], [Deprecation].
- You may optionally append "(by @<author>)" at the end of the line, where "<author>" is either one (just one)
of your GitHub username, real name or affiliated organization. These affiliations will be displayed in
the release notes for every release.
-->

- [Improvement] Remove the need to run ``npm install`` when setting up an edx-platform development environment. (by @kdmccormick)
13 changes: 13 additions & 0 deletions tutor/templates/apps/openedx/settings/partials/common_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,5 +219,18 @@
"user": None,
}

# Upstream expects node_modules to be within edx-platform, but we put
# node_modules under /openedx/node_modules, so we must adjust any settings
# that hold a node_modules path.
NODE_MODULES_ROOT = "/openedx/node_modules/@edx"
STATICFILES_DIRS = [
*[
staticfiles_dir for staticfiles_dir in STATICFILES_DIRS
if "node_modules/@edx" not in staticfiles_dir
],
NODE_MODULES_ROOT,
]
PIPELINE["UGLIFYJS_BINARY"] = "/openedx/node_modules/.bin/uglifyjs"

{{ patch("openedx-common-settings") }}
######## End of settings common to LMS and CMS
12 changes: 6 additions & 6 deletions tutor/templates/build/openedx/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ ENV PATH /openedx/nodeenv/bin:/openedx/venv/bin:${PATH}
RUN pip install nodeenv==1.7.0
RUN nodeenv /openedx/nodeenv --node=16.14.0 --prebuilt

# Install nodejs requirements
# Install nodejs requirements to /openedx/node_modules
ARG NPM_REGISTRY={{ NPM_REGISTRY }}
COPY --from=code /openedx/edx-platform/package.json /openedx/edx-platform/package.json
COPY --from=code /openedx/edx-platform/package-lock.json /openedx/edx-platform/package-lock.json
WORKDIR /openedx/edx-platform
COPY --from=code /openedx/edx-platform/package.json /openedx/package.json
COPY --from=code /openedx/edx-platform/package-lock.json /openedx/package-lock.json
WORKDIR /openedx
RUN npm install --verbose --registry=$NPM_REGISTRY

###### Production image with system and python requirements
Expand All @@ -136,9 +136,9 @@ COPY --chown=app:app --from=python /opt/pyenv /opt/pyenv
COPY --chown=app:app --from=python-requirements /openedx/venv /openedx/venv
COPY --chown=app:app --from=python-requirements /openedx/requirements /openedx/requirements
COPY --chown=app:app --from=nodejs-requirements /openedx/nodeenv /openedx/nodeenv
COPY --chown=app:app --from=nodejs-requirements /openedx/edx-platform/node_modules /openedx/edx-platform/node_modules
COPY --chown=app:app --from=nodejs-requirements /openedx/node_modules /openedx/node_modules

ENV PATH /openedx/venv/bin:./node_modules/.bin:/openedx/nodeenv/bin:${PATH}
ENV PATH /openedx/venv/bin:/openedx/node_modules/.bin:/openedx/nodeenv/bin:${PATH}
ENV VIRTUAL_ENV /openedx/venv/
WORKDIR /openedx/edx-platform

Expand Down
Loading

0 comments on commit 45e9e40

Please sign in to comment.