Skip to content

Commit

Permalink
build: commit xmodule Webpack config and delete xmodule_assets script
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 committed Jul 11, 2023
1 parent 5c867ed commit f93ab1d
Show file tree
Hide file tree
Showing 26 changed files with 163 additions and 510 deletions.
21 changes: 9 additions & 12 deletions docs/decisions/0017-reimplement-asset-processing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
- ``scripts/build-assets.sh``

A Bash script that contains all build stages, with subcommands available for running each stage separately. Its command-line interface inspired by Tutor's ``openedx-assets`` script. The script will be runnable on any POSIX system, including macOS and Ubuntu and it will linted for common shell scripting mistakes using `shellcheck <https://www.shellcheck.net>`_.

* - + **Build stage 1: Copy npm-installed assets** from node_modules to other folders in edx-platform. They are used by certain especially-old legacy LMS & CMS frontends that are not set up to work with npm directly.

- ``paver update_assets --skip-collect``
Expand All @@ -141,7 +141,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
- ``scripts/build-assets.sh npm``

Pure Bash reimplementation. See *Rejected Alternatives* for a note about this.

* - + **Build stage 2: Copy XModule fragments** from the xmodule source tree over to input directories for Webpack and SCSS compilation. This is required for a hard-coded list of old XModule-style XBlocks. This is not required for new pure XBlocks, which include (or pip-install) their assets into edx-platform as ready-to-serve JS/CSS/etc fragments.

- ``paver process_xmodule_assets``, or
Expand All @@ -150,13 +150,10 @@ The three top-level edx-platform asset processing actions are *build*, *collect*

Equivalent paver task and console script, both pointing at to an application-level Python module. That module inspects attributes from legacy XModule-style XBlock classes in order to determine which static assets to copy and what to name them.

- ``scripts/build-assets.sh xmodule``
- (step no longer needed)

Initially, this command will just call out to the existing ``xmodule_assets`` command. Eventually, in order to make this step Python-free, we will need do either one or both of the following:
We will `remove the need for this step entirely <https://github.com/openedx/edx-platform/issues/31624>`_.

+ `Reimplement this step in Bash <https://github.com/openedx/edx-platform/issues/31611>`_.
+ `Remove the need for this step entirely <https://github.com/openedx/edx-platform/issues/31624>`_.

* - + **Build stage 3: Run Webpack** in order to to shim, minify, otherwise process, and bundle JS modules. This requires a call to the npm-installed ``webpack`` binary.

- ``paver webpack``
Expand All @@ -168,7 +165,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
Bash wrapper around a call to webpack. The script will accept parameters rather than looking up Django settings itself.

The print_setting command will still be available for distributions to use to extract ``STATIC_ROOT`` from Django settings, but it will only need to be run once. As described in **Build Configuration** below, unnecessary Django settings will be removed. Some distributions may not even need to look up ``STATIC_ROOT``; Tutor, for example, will probably render ``STATIC_ROOT`` directly into the environment variable ``OPENEDX_BUILD_ASSETS_OPTS`` variable, described in the **Build Configuration**.

* - + **Build stage 4: Compile default SCSS** into CSS for legacy LMS/CMS frontends.

- ``paver compile_sass``
Expand All @@ -180,7 +177,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
- ``scripts/build-assets.sh css``

Bash reimplementation, calling ``node-sass`` and ``rtlcss``.

The initial implementation of build-assets.sh may use ``sassc``, a CLI provided by libsass, instead of node-sass. Then, ``sassc`` can be replaced by ``node-sass`` as part of a subsequent `edx-platform frontend framework upgrade effort <https://github.com/openedx/edx-platform/issues/31616>`_.

* - + **Build stage 5: Compile themes' SCSS** into CSS for legacy LMS/CMS frontends. The default SCSS is used as a base, and theme-provided SCSS files are used as overrides. Themes are searched for from some number of operator-specified theme directories.
Expand All @@ -198,7 +195,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
The management command will remain available, but it will need to be updated to point at the Bash script, which will replace the paver task (see build stage 4 for details).

The overall asset *build* action will use the Bash script; this means that list of theme directories will need to be provided as arguments, but it ensures that the build can remain Python-free.

* - **Collect** the built static assets from edx-platform to another location (the ``STATIC_ROOT``) so that they can be efficiently served *without* Django's webserver. This step, by nature, requires Python and Django in order to find and organize the assets, which may come from edx-platform itself or from its many installed Python and NPM packages. This is only needed for **production** environments, where it is usually desirable to serve assets with something efficient like NGINX.

- ``paver update_assets``
Expand All @@ -210,7 +207,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
- ``./manage.py lms collectstatic --noinput && ./manage.py cms collectstatic --noinput``

The standard Django interface will be used without a wrapper. The ignore patterns will be added to edx-platform's `staticfiles app configuration <https://docs.djangoproject.com/en/4.1/ref/contrib/staticfiles/#customizing-the-ignored-pattern-list>`_ so that they do not need to be supplied as part of the command.

* - **Watch** static assets for changes in the background. When a change occurs, rebuild them automatically, so that the Django webserver picks up the changes. This is only necessary in **development** environments. A few different sets of assets may be watched: XModule fragments, Webpack assets, default SCSS, and theme SCSS.

- ``paver watch_assets``
Expand Down Expand Up @@ -302,7 +299,7 @@ Either way, the migration path is straightforward:
* - ``openedx-assets npm``
- ``scripts/build-assets.sh npm``
* - ``openedx-assets xmodule``
- ``scripts/build-assets.sh xmodule``
- (no longer needed)
* - ``openedx-assets common``
- ``scripts/build-assets.sh css``
* - ``openedx-assets themes``
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/tests/test_discussion_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def test_has_permission(self):
"""
permission_canary = object()
with mock.patch(
'lms.djangoapps.discussion.django_comment_client.permissions.has_permission',
'xmodule.discussion_block.has_permission',
return_value=permission_canary,
) as has_perm:
actual_permission = self.block.has_permission("test_permission")
Expand Down
53 changes: 3 additions & 50 deletions pavelib/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,6 @@ class SassWatcher(PatternMatchingEventHandler):
"""
ignore_directories = True
patterns = ['*.scss']
ignore_patterns = ['common/static/xmodule/*']

def register(self, observer, directories):
"""
Expand Down Expand Up @@ -385,47 +384,6 @@ def on_any_event(self, event):
traceback.print_exc()


class XModuleSassWatcher(SassWatcher):
"""
Watches for sass file changes
"""
ignore_directories = True
ignore_patterns = []

@debounce()
def on_any_event(self, event):
print('\tCHANGED:', event.src_path)
try:
process_xmodule_assets()
except Exception: # pylint: disable=broad-except
traceback.print_exc()


class XModuleAssetsWatcher(PatternMatchingEventHandler):
"""
Watches for css and js file changes
"""
ignore_directories = True
patterns = ['*.css', '*.js']

def register(self, observer):
"""
Register files with observer
"""
observer.schedule(self, 'xmodule/', recursive=True)

@debounce()
def on_any_event(self, event):
print('\tCHANGED:', event.src_path)
try:
process_xmodule_assets()
except Exception: # pylint: disable=broad-except
traceback.print_exc()

# To refresh the hash values of static xmodule content
restart_django_servers()


@task
@no_help
@cmdopts([
Expand Down Expand Up @@ -701,16 +659,13 @@ def copy_vendor_library_dir(library_dir, skip_if_missing=False):


@task
@needs(
'pavelib.prereqs.install_python_prereqs',
)
@no_help
def process_xmodule_assets():
"""
Process XModule static assets.
"""
sh('xmodule_assets common/static/xmodule')
print("\t\tFinished processing xmodule assets.")
print("\t\tProcessing xmodule assets is no longer needed. This task is now a no-op.")
print("\t\tWhen paver is removed from edx-platform, this step will not replaced.")


def restart_django_servers():
Expand Down Expand Up @@ -908,8 +863,6 @@ def watch_assets(options):
observer = Observer(timeout=wait)

SassWatcher().register(observer, sass_directories)
XModuleSassWatcher().register(observer, ['xmodule/'])
XModuleAssetsWatcher().register(observer)

print("Starting asset watcher...")
observer.start()
Expand All @@ -931,6 +884,7 @@ def watch_assets(options):
@task
@needs(
'pavelib.prereqs.install_node_prereqs',
'pavelib.prereqs.install_python_prereqs',
)
@consume_args
@timed
Expand Down Expand Up @@ -982,7 +936,6 @@ def update_assets(args):
args = parser.parse_args(args)
collect_log_args = {}

process_xmodule_assets()
process_npm_assets()

# Build Webpack
Expand Down
1 change: 0 additions & 1 deletion pavelib/js_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
@needs(
'pavelib.prereqs.install_node_prereqs',
'pavelib.utils.test.utils.clean_reports_dir',
'pavelib.assets.process_xmodule_assets',
)
@cmdopts([
("suite=", "s", "Test suite to run"),
Expand Down
2 changes: 0 additions & 2 deletions pavelib/paver_tests/test_servers.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ def verify_server_task(self, task_name, options):
expected_asset_settings = "test_static_optimized"
expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS
if not is_fast:
expected_messages.append("xmodule_assets common/static/xmodule")
expected_messages.append("install npm_assets")
expected_messages.extend(
[c.format(settings=expected_asset_settings,
Expand Down Expand Up @@ -275,7 +274,6 @@ def verify_run_all_servers_task(self, options):
expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS
expected_messages = []
if not is_fast:
expected_messages.append("xmodule_assets common/static/xmodule")
expected_messages.append("install npm_assets")
expected_messages.extend(
[c.format(settings=expected_asset_settings,
Expand Down
3 changes: 0 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,5 @@
],
'xblock.v1': XBLOCKS,
'xblock_asides.v1': XBLOCKS_ASIDES,
'console_scripts': [
'xmodule_assets = xmodule.static_content:main',
],
}
)
2 changes: 1 addition & 1 deletion webpack.common.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var StringReplace = require('string-replace-webpack-plugin');
var Merge = require('webpack-merge');

var files = require('./webpack-config/file-lists.js');
var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js');
var xmoduleJS = require('./webpack.xmodule.config.js');

var filesWithRequireJSBlocks = [
path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'),
Expand Down
136 changes: 136 additions & 0 deletions webpack.xmodule.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
module.exports = {
"entry": {
"AboutBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/display.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/collapsible.js",
"./xmodule/js/src/html/imageModal.js",
"./xmodule/js/common_static/js/vendor/draggabilly.js"
],
"AboutBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/edit.js"
],
"AnnotatableBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/display.js",
"./xmodule/js/src/annotatable/display.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/collapsible.js"
],
"AnnotatableBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/raw/edit/xml.js"
],
"ConditionalBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/conditional/display.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/collapsible.js"
],
"ConditionalBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/sequence/edit.js"
],
"CourseInfoBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/display.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/collapsible.js",
"./xmodule/js/src/html/imageModal.js",
"./xmodule/js/common_static/js/vendor/draggabilly.js"
],
"CourseInfoBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/edit.js"
],
"CustomTagBlockDisplay": "./xmodule/js/src/xmodule.js",
"CustomTagBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/raw/edit/xml.js"
],
"HtmlBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/display.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/collapsible.js",
"./xmodule/js/src/html/imageModal.js",
"./xmodule/js/common_static/js/vendor/draggabilly.js"
],
"HtmlBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/edit.js"
],
"LTIBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/lti/lti.js"
],
"LTIBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/raw/edit/metadata-only.js"
],
"LibraryContentBlockDisplay": "./xmodule/js/src/xmodule.js",
"LibraryContentBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/vertical/edit.js"
],
"PollBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/poll/poll.js",
"./xmodule/js/src/poll/poll_main.js"
],
"PollBlockEditor": "./xmodule/js/src/xmodule.js",
"ProblemBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/capa/display.js",
"./xmodule/js/src/collapsible.js",
"./xmodule/js/src/capa/imageinput.js",
"./xmodule/js/src/capa/schematic.js"
],
"ProblemBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/problem/edit.js"
],
"SequenceBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/sequence/display.js"
],
"SequenceBlockEditor": "./xmodule/js/src/xmodule.js",
"SplitTestBlockDisplay": "./xmodule/js/src/xmodule.js",
"SplitTestBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/sequence/edit.js"
],
"StaticTabBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/display.js",
"./xmodule/js/src/javascript_loader.js",
"./xmodule/js/src/collapsible.js",
"./xmodule/js/src/html/imageModal.js",
"./xmodule/js/common_static/js/vendor/draggabilly.js"
],
"StaticTabBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/html/edit.js"
],
"VideoBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/video/10_main.js"
],
"VideoBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/tabs/tabs-aggregator.js"
],
"WordCloudBlockDisplay": [
"./xmodule/js/src/xmodule.js",
"./xmodule/assets/word_cloud/src/js/word_cloud.js"
],
"WordCloudBlockEditor": [
"./xmodule/js/src/xmodule.js",
"./xmodule/js/src/raw/edit/metadata-only.js"
]
}
};
Loading

0 comments on commit f93ab1d

Please sign in to comment.