Skip to content

Commit

Permalink
build: include built-in XBlock JS directly rather than copying it
Browse files Browse the repository at this point in the history 
TODO: will copy in commit message from PR when squash merging

Part of: #32481
  • Loading branch information
@kdmccormick
kdmccormick committed Jul 26, 2023
1 parent 3e67719 commit e87b57d
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 135 deletions.
2 changes: 0 additions & 2 deletions conf/locale/config.yaml
View file
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
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
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
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

0 comments on commit e87b57d

Please sign in to comment.