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

[2/3] build: include built-in XBlock JS directly rather than copying it #32480

Merged
merged 1 commit into from
Jul 26, 2023
Merged

[2/3] build: include built-in XBlock JS directly rather than copying it #32480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions conf/locale/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ dummy_locales:

# Directories we don't search for strings.
ignore_dirs:
- common/static/xmodule/modules
- common/static/xmodule/descriptors
# Directories with no user-facing code.
- '*/migrations'
- '*/envs'
Expand Down
4 changes: 1 addition & 3 deletions webpack-config/file-lists.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ module.exports = {
path.resolve(__dirname, '../common/static/common/js/components/views/paging_footer.js'),
path.resolve(__dirname, '../cms/static/js/views/paging.js'),
path.resolve(__dirname, '../common/static/common/js/components/utils/view_utils.js'),
/descriptors\/js/,
/modules\/js/,
/xmodule\/js\/src\//,
/xmodule\/js\/src/,
path.resolve(__dirname, '../openedx/features/course_bookmarks/static/course_bookmarks/js/views/bookmark_button.js')
],

Expand Down
118 changes: 113 additions & 5 deletions webpack.common.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js');

var filesWithRequireJSBlocks = [
path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'),
/descriptors\/js/,
/modules\/js/,
/xmodule\/js\/src\//
/xmodule\/js\/src/
];

var defineHeader = /\(function ?\(((define|require|requirejs|\$)(, )?)+\) ?\{/;
Expand Down Expand Up @@ -311,14 +309,124 @@ module.exports = Merge.smart({
test: /xblock\/runtime.v1/,
loader: 'exports-loader?window.XBlock!imports-loader?XBlock=xblock/core,this=>window'
},
/*******************************************************************************************************
/* BUILT-IN XBLOCK ASSETS WITH GLOBAL DEFINITIONS:
*
* The monstrous list of globally-namespace modules below is the result of a JS build refactoring.
* Originally, all of these modules were copied to common/static/xmodule/js/[module|descriptors]/, and
* this file simply contained the lines:
*
* {
* test: /descriptors\/js/,
* loader: 'imports-loader?this=>window'
* },
* {
* test: /modules\/js/,
* loader: 'imports-loader?this=>window'
* },
*
* We removed that asset copying because it added complexity to the build, but as a result, in order to
* preserve exact parity with the preexisting global namespace, we had to enumerate all formely-copied
* modules here. It is very likely that many of these modules do not need to be in this list. Future
* refactorings are welcome to try to prune the list down to the minimal set of modules. As far as
* we know, the only modules that absolutely need to be added to the global namespace are those
* which define module types, for example "Problem" (in xmodule/js/src/capa/display.js).
*/
{
test: /descriptors\/js/,
test: /xmodule\/assets\/word_cloud\/src\/js\/word_cloud.js/,
loader: 'imports-loader?this=>window'
},
{
test: /modules\/js/,
test: /xmodule\/js\/common_static\/js\/vendor\/draggabilly.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/annotatable\/display.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/capa\/display.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/capa\/imageinput.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/capa\/schematic.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/collapsible.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/conditional\/display.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/html\/display.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/html\/edit.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/html\/imageModal.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/javascript_loader.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/lti\/lti.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/poll\/poll.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/poll\/poll_main.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/problem\/edit.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/raw\/edit\/metadata-only.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/raw\/edit\/xml.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/sequence\/display.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/sequence\/edit.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/tabs\/tabs-aggregator.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/vertical\/edit.js/,
loader: 'imports-loader?this=>window'
},
{
test: /xmodule\/js\/src\/video\/10_main.js/,
loader: 'imports-loader?this=>window'
},
/*
* END BUILT-IN XBLOCK ASSETS WITH GLOBAL DEFINITIONS
******************************************************************************************************/
{
test: /codemirror/,
loader: 'exports-loader?window.CodeMirror'
Expand Down
4 changes: 1 addition & 3 deletions xmodule/assets/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and

* For many older blocks, their JS is:

* copied to ``common/static/xmodule`` by `static_content.py`_ (aka ``xmodule_assets``),
* then bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``,
* bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``,
* which is included into `webpack.common.config.js`_,
* allowing it to be included into XBlock fragments using ``add_webpack_js_to_fragment`` from `builtin_assets.py`_.

Expand All @@ -77,7 +76,6 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and

As part of an `active build refactoring`_:

* We update the older builtin XBlocks to reference their JS directly rather than using copies of it.
* We will move ``webpack.xmodule.config.js`` here instead of generating it.
* We will consolidate all edx-platform XBlock JS here in `xmodule/assets`_.
* We will delete the ``xmodule_assets`` script.
Expand Down
Loading