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.

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

build: npm run compile-sass (WIP) #32722

Closed
Closed
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
build: commit xmodule Webpack config and delete xmodule_assets script
TODO: will copy in commit message from PR when squash-merging

Part of: #32481
kdmccormick committed Jul 18, 2023
commit 38cca730dd67f9739d5ffc5874eda0df8311d107
9 changes: 3 additions & 6 deletions docs/decisions/0017-reimplement-asset-processing.rst
Original file line number Diff line number Diff line change
@@ -150,12 +150,9 @@ 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:

+ `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>`_.
We will `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.

@@ -302,7 +299,7 @@ Either way, the migration path is straightforward:
* - ``openedx-assets npm``
- ``scripts/copy-node-modules.sh # (automatically invoked by 'npm install'!)
* - ``openedx-assets xmodule``
- ``scripts/build-assets.sh xmodule``
- (no longer needed)
* - ``openedx-assets common``
- ``scripts/build-assets.sh css``
* - ``openedx-assets themes``
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/tests/test_discussion_xblock.py
Original file line number Diff line number Diff line change
@@ -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")
54 changes: 3 additions & 51 deletions pavelib/assets.py
Original file line number Diff line number Diff line change
@@ -323,7 +323,6 @@ class SassWatcher(PatternMatchingEventHandler):
"""
ignore_directories = True
patterns = ['*.scss']
ignore_patterns = ['common/static/xmodule/*']

def register(self, observer, directories):
"""
@@ -352,47 +351,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([
@@ -615,16 +573,13 @@ def process_npm_assets():


@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():
@@ -822,8 +777,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()
@@ -845,6 +798,7 @@ def watch_assets(options):
@task
@needs(
'pavelib.prereqs.install_node_prereqs',
'pavelib.prereqs.install_python_prereqs',
)
@consume_args
@timed
@@ -896,8 +850,6 @@ def update_assets(args):
args = parser.parse_args(args)
collect_log_args = {}

process_xmodule_assets()

# Build Webpack
call_task('pavelib.assets.webpack', options={'settings': args.settings})

1 change: 0 additions & 1 deletion pavelib/js_test.py
Original file line number Diff line number Diff line change
@@ -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"),
2 changes: 0 additions & 2 deletions pavelib/paver_tests/test_servers.py
Original file line number Diff line number Diff line change
@@ -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,
@@ -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,
3 changes: 0 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
@@ -186,8 +186,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
@@ -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'),
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"
]
}
};
19 changes: 0 additions & 19 deletions xmodule/annotatable_block.py
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@
import textwrap

from lxml import etree
from pkg_resources import resource_filename
from web_fragments.fragment import Fragment
from xblock.core import XBlock
from xblock.fields import Scope, String
@@ -15,7 +14,6 @@
from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment
from xmodule.xml_block import XmlMixin
from xmodule.x_module import (
HTMLSnippet,
ResourceTemplates,
shim_xmodule_js,
XModuleMixin,
@@ -35,7 +33,6 @@ class AnnotatableBlock(
XmlMixin,
EditingMixin,
XModuleToXBlockMixin,
HTMLSnippet,
ResourceTemplates,
XModuleMixin,
):
@@ -73,22 +70,6 @@ class AnnotatableBlock(

uses_xmodule_styles_setup = True

preview_view_js = {
'js': [
resource_filename(__name__, 'js/src/html/display.js'),
resource_filename(__name__, 'js/src/annotatable/display.js'),
resource_filename(__name__, 'js/src/javascript_loader.js'),
resource_filename(__name__, 'js/src/collapsible.js'),
],
'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'),
}

studio_view_js = {
'js': [
resource_filename(__name__, 'js/src/raw/edit/xml.js'),
],
'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'),
}
studio_js_module_name = "XMLEditingDescriptor"
mako_template = "widgets/raw-edit.html"

13 changes: 5 additions & 8 deletions xmodule/assets/README.rst
Original file line number Diff line number Diff line change
@@ -41,8 +41,8 @@ the corresponding folders in any enabled themes, as part of the edx-platform bui
It is collected into the static root, and then linked to from XBlock fragments by the
``add_sass_to_fragment`` function in `builtin_assets.py`_.

.. _AnnotatableBlockDisplay: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss
.. _AnnotatableBlockEditor: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss
.. _AnnotatableBlockDisplay.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss
.. _AnnotatableBlockEditor.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss
.. _annotatable/_display.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/annotatable/_display.scss
.. _simplify things: https://github.com/openedx/edx-platform/issues/32621

@@ -59,7 +59,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and

* For many older blocks, their JS is:

* bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``,
* bundled using a `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`_.

@@ -74,11 +74,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and
* `VerticalBlock`_
* `LibrarySourcedBlock`_

As part of an `active build refactoring`_:

* 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.
As part of an `active build refactoring`_, we will soon consolidate all edx-platform XBlock JS here in `xmodule/assets`_.

.. _xmodule/assets: https://github.com/openedx/edx-platform/tree/master/xmodule/assets
.. _xmodule/js: https://github.com/openedx/edx-platform/tree/master/xmodule/js
@@ -91,4 +87,5 @@ As part of an `active build refactoring`_:
.. _builtin_assets.py: https://github.com/openedx/edx-platform/tree/master/xmodule/util/builtin_assets.py
.. _static_content.py: https://github.com/openedx/edx-platform/blob/master/xmodule/static_content.py
.. _library_source_block/style.css: https://github.com/openedx/edx-platform/blob/master/xmodule/assets/library_source_block/style.css
.. _webpack.xmodule.config.js: https://github.com/openedx/edx-platform/blob/master/webpack.xmodule.config.js
.. _webpack.common.config.js: https://github.com/openedx/edx-platform/blob/master/webpack.common.config.js
Loading