[3/3] build: commit builtinblocks Webpack config and stub out xmodule_assets - #32685
Merged
kdmccormick merged 2 commits intoJul 27, 2023
Merged
Conversation
This was referenced Jul 7, 2023
kdmccormick
changed the title
xmodule_assets script xmodule_assets script
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
5 times, most recently
from
July 8, 2023 01:07
1f13076 to
ff3ab4d
Compare
kdmccormick
marked this pull request as ready for review
July 10, 2023 21:26
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
2 times, most recently
from
July 11, 2023 15:21
6666750 to
f93ab1d
Compare
kdmccormick
marked this pull request as draft
July 11, 2023 18:57
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
2 times, most recently
from
July 21, 2023 02:40
38cca73 to
6b38d03
Compare
This was referenced Jul 24, 2023
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
2 times, most recently
from
July 25, 2023 14:57
9cf9845 to
26ee588
Compare
kdmccormick
changed the title
xmodule_assets script xmodule_assets script
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
2 times, most recently
from
July 25, 2023 15:10
d427724 to
02d90e3
Compare
kdmccormick
changed the title
xmodule_assets script xmodule_assets
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
from
July 25, 2023 15:24
02d90e3 to
04a4eda
Compare
kdmccormick
marked this pull request as ready for review
July 25, 2023 15:26
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
from
July 25, 2023 15:38
04a4eda to
fad86dc
Compare
feanil
approved these changes
Jul 25, 2023
kdmccormick
added a commit
that referenced
this pull request
Jul 26, 2023
This PR implements much of the static assets rework ADR [1], including: * `npm run build[-dev]`, and its subcommands, * `npm run webpack[-dev]` and * `npm run compile-sass[-dev]`. This is backwards-compatible. `paver update_assets` should not be affected. The new command warns that it is "experimental" for a few reasons: * `npm run build` will fail in the webpack phase unless you first run `xmodule_assets`. This will be changed soon [2]. * We have tested the new build, but not quite so thoroughly that we'd recommend it as the production default yet. Once the xmodule_assets work lands, we'll share this on the forums so early adopters can try it out. * The commands lack some top-level documentation. Once they stabilize more, we'll add a section to the README that explains how and when to use `npm run build` and its subcommands and its env vars. * `npm run watch` is not yet implemented. References: 1. https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst 2. #32685 Part of: #31604
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
from
July 26, 2023 21:40
fad86dc to
73f28cc
Compare
The Webpack configuration file for built-in XBlock JS used to be generated at build time and git-ignored. It lived at common/static/xmodule/webpack.xmodule.config.js. It was generated because the JS that it referred to was also generated at build-time, and the filenames of those JS modules were not static. Now that its contents have been made entirely static [1], there is no reason we need to continue generating this Webpack configuration file. So, we check it into edx-platform under the name ./webpack.builtinblocks.config.js. We choose to put it in the repo's root directory because the paths contained in the config file are relative to the repo's root. This allows us to behead both the xmodule/static_content.py (`xmodule_assets`) script andthe `process_xmodule_assets` paver task, a major step in removing the need for Python in the edx-platform asset build [2]. It also allows us to delete the `HTMLSnippet` class and all associated attributes, which were exclusively used by xmodule/static_content.py.. We leave `xmodule_assets` and `process_xmodule_assets` in as stubs for now in order to avoid breaking external code (like Tutor) which calls Paver; the entire pavelib/assets.py function will be eventually removed soon anyway [3]. Further, to avoid extraneous refactoring, we keep one method of `HTMLSnippet` around on a few of its former subclasses: `get_html`. This method was originally part of the XModule framework; now, it is left over on a few classes as a simple internal helper method. References: 1. openedx#32480 2. openedx#31800 3. openedx#31895 Part of: openedx#32481
kdmccormick
force-pushed
the
kdmccormick/xmodule-js-commit
branch
from
July 27, 2023 13:26
73f28cc to
7eb4836
Compare
kdmccormick
enabled auto-merge (squash)
July 27, 2023 14:15
Contributor
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
Contributor
|
2U Release Notice: This PR has been deployed to the edX production environment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of a series of PRs:
The previous one is:
To review this PR, look at just the latest commit (the one matching the PR title).
Description and supporting info
The Webpack configuration file for built-in XBlock JS used to be generated at build time and git-ignored. It lived at common/static/xmodule/webpack.xmodule.config.js. It was generated because the JS that it referred to was also generated at build-time, and the filenames of those JS modules were not static.
Now that its contents have been made entirely static [1], there is no reason we need to continue generating this Webpack configuration file. So, we check it into edx-platform under the name ./webpack.builtinblocks.config.js. We choose to put it in the repo's root directory because the paths contained in the config file are relative to the repo's root.
This allows us to behead both the xmodule/static_content.py (
xmodule_assets) script andtheprocess_xmodule_assetspaver task, a major step in removing the need for Python in the edx-platform asset build [2]. It also allows us to delete theHTMLSnippetclass and all associated attributes, which were exclusively used by xmodule/static_content.py..We leave
xmodule_assetsandprocess_xmodule_assetsin as stubs for now in order to avoid breaking external code (like Tutor) which calls Paver; the entire pavelib/assets.py function will be eventually removed soon anyway [3]. Further, to avoid extraneous refactoring, we keep one method ofHTMLSnippetaround on a few of its former subclasses:get_html. This method was originally part of the XModule framework; now, it is left over on a few classes as a simple internal helper method.References:
Testing
Follow the same instructions from #32480, but sub in this branch.