Skip to content

Commit 3557799

Browse files
kdmccormickkdmccormick
authored
build: commit builtinblocks Webpack config and stub out xmodule_assets (openedx#32685)
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
1 parent c0e9dc9 commit 3557799

24 files changed

+164
-481
lines changed

‎lms/djangoapps/courseware/tests/test_discussion_xblock.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ def test_has_permission(self):
221221
"""
222222
permission_canary = object()
223223
with mock.patch(
224-
'lms.djangoapps.discussion.django_comment_client.permissions.has_permission',
224+
'xmodule.discussion_block.has_permission',
225225
return_value=permission_canary,
226226
) as has_perm:
227227
actual_permission = self.block.has_permission("test_permission")

‎pavelib/assets.py

+3-51
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,6 @@ class SassWatcher(PatternMatchingEventHandler):
323323
"""
324324
ignore_directories = True
325325
patterns = ['*.scss']
326-
ignore_patterns = ['common/static/xmodule/*']
327326

328327
def register(self, observer, directories):
329328
"""
@@ -352,47 +351,6 @@ def on_any_event(self, event):
352351
traceback.print_exc()
353352

354353

355-
class XModuleSassWatcher(SassWatcher):
356-
"""
357-
Watches for sass file changes
358-
"""
359-
ignore_directories = True
360-
ignore_patterns = []
361-
362-
@debounce()
363-
def on_any_event(self, event):
364-
print('\tCHANGED:', event.src_path)
365-
try:
366-
process_xmodule_assets()
367-
except Exception: # pylint: disable=broad-except
368-
traceback.print_exc()
369-
370-
371-
class XModuleAssetsWatcher(PatternMatchingEventHandler):
372-
"""
373-
Watches for css and js file changes
374-
"""
375-
ignore_directories = True
376-
patterns = ['*.css', '*.js']
377-
378-
def register(self, observer):
379-
"""
380-
Register files with observer
381-
"""
382-
observer.schedule(self, 'xmodule/', recursive=True)
383-
384-
@debounce()
385-
def on_any_event(self, event):
386-
print('\tCHANGED:', event.src_path)
387-
try:
388-
process_xmodule_assets()
389-
except Exception: # pylint: disable=broad-except
390-
traceback.print_exc()
391-
392-
# To refresh the hash values of static xmodule content
393-
restart_django_servers()
394-
395-
396354
@task
397355
@no_help
398356
@cmdopts([
@@ -615,16 +573,13 @@ def process_npm_assets():
615573

616574

617575
@task
618-
@needs(
619-
'pavelib.prereqs.install_python_prereqs',
620-
)
621576
@no_help
622577
def process_xmodule_assets():
623578
"""
624579
Process XModule static assets.
625580
"""
626-
sh('xmodule_assets common/static/xmodule')
627-
print("\t\tFinished processing xmodule assets.")
581+
print("\t\tProcessing xmodule assets is no longer needed. This task is now a no-op.")
582+
print("\t\tWhen paver is removed from edx-platform, this step will not replaced.")
628583

629584

630585
def restart_django_servers():
@@ -822,8 +777,6 @@ def watch_assets(options):
822777
observer = Observer(timeout=wait)
823778

824779
SassWatcher().register(observer, sass_directories)
825-
XModuleSassWatcher().register(observer, ['xmodule/'])
826-
XModuleAssetsWatcher().register(observer)
827780

828781
print("Starting asset watcher...")
829782
observer.start()
@@ -845,6 +798,7 @@ def watch_assets(options):
845798
@task
846799
@needs(
847800
'pavelib.prereqs.install_node_prereqs',
801+
'pavelib.prereqs.install_python_prereqs',
848802
)
849803
@consume_args
850804
@timed
@@ -896,8 +850,6 @@ def update_assets(args):
896850
args = parser.parse_args(args)
897851
collect_log_args = {}
898852

899-
process_xmodule_assets()
900-
901853
# Build Webpack
902854
call_task('pavelib.assets.webpack', options={'settings': args.settings})
903855

‎pavelib/js_test.py

-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
@needs(
2727
'pavelib.prereqs.install_node_prereqs',
2828
'pavelib.utils.test.utils.clean_reports_dir',
29-
'pavelib.assets.process_xmodule_assets',
3029
)
3130
@cmdopts([
3231
("suite=", "s", "Test suite to run"),

‎pavelib/paver_tests/test_servers.py

-2
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,6 @@ def verify_server_task(self, task_name, options):
214214
expected_asset_settings = "test_static_optimized"
215215
expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS
216216
if not is_fast:
217-
expected_messages.append("xmodule_assets common/static/xmodule")
218217
expected_messages.append("install npm_assets")
219218
expected_messages.extend(
220219
[c.format(settings=expected_asset_settings,
@@ -259,7 +258,6 @@ def verify_run_all_servers_task(self, options):
259258
expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS
260259
expected_messages = []
261260
if not is_fast:
262-
expected_messages.append("xmodule_assets common/static/xmodule")
263261
expected_messages.append("install npm_assets")
264262
expected_messages.extend(
265263
[c.format(settings=expected_asset_settings,

‎webpack.builtinblocks.config.js

+136
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
module.exports = {
2+
"entry": {
3+
"AboutBlockDisplay": [
4+
"./xmodule/js/src/xmodule.js",
5+
"./xmodule/js/src/html/display.js",
6+
"./xmodule/js/src/javascript_loader.js",
7+
"./xmodule/js/src/collapsible.js",
8+
"./xmodule/js/src/html/imageModal.js",
9+
"./xmodule/js/common_static/js/vendor/draggabilly.js"
10+
],
11+
"AboutBlockEditor": [
12+
"./xmodule/js/src/xmodule.js",
13+
"./xmodule/js/src/html/edit.js"
14+
],
15+
"AnnotatableBlockDisplay": [
16+
"./xmodule/js/src/xmodule.js",
17+
"./xmodule/js/src/html/display.js",
18+
"./xmodule/js/src/annotatable/display.js",
19+
"./xmodule/js/src/javascript_loader.js",
20+
"./xmodule/js/src/collapsible.js"
21+
],
22+
"AnnotatableBlockEditor": [
23+
"./xmodule/js/src/xmodule.js",
24+
"./xmodule/js/src/raw/edit/xml.js"
25+
],
26+
"ConditionalBlockDisplay": [
27+
"./xmodule/js/src/xmodule.js",
28+
"./xmodule/js/src/conditional/display.js",
29+
"./xmodule/js/src/javascript_loader.js",
30+
"./xmodule/js/src/collapsible.js"
31+
],
32+
"ConditionalBlockEditor": [
33+
"./xmodule/js/src/xmodule.js",
34+
"./xmodule/js/src/sequence/edit.js"
35+
],
36+
"CourseInfoBlockDisplay": [
37+
"./xmodule/js/src/xmodule.js",
38+
"./xmodule/js/src/html/display.js",
39+
"./xmodule/js/src/javascript_loader.js",
40+
"./xmodule/js/src/collapsible.js",
41+
"./xmodule/js/src/html/imageModal.js",
42+
"./xmodule/js/common_static/js/vendor/draggabilly.js"
43+
],
44+
"CourseInfoBlockEditor": [
45+
"./xmodule/js/src/xmodule.js",
46+
"./xmodule/js/src/html/edit.js"
47+
],
48+
"CustomTagBlockDisplay": "./xmodule/js/src/xmodule.js",
49+
"CustomTagBlockEditor": [
50+
"./xmodule/js/src/xmodule.js",
51+
"./xmodule/js/src/raw/edit/xml.js"
52+
],
53+
"HtmlBlockDisplay": [
54+
"./xmodule/js/src/xmodule.js",
55+
"./xmodule/js/src/html/display.js",
56+
"./xmodule/js/src/javascript_loader.js",
57+
"./xmodule/js/src/collapsible.js",
58+
"./xmodule/js/src/html/imageModal.js",
59+
"./xmodule/js/common_static/js/vendor/draggabilly.js"
60+
],
61+
"HtmlBlockEditor": [
62+
"./xmodule/js/src/xmodule.js",
63+
"./xmodule/js/src/html/edit.js"
64+
],
65+
"LTIBlockDisplay": [
66+
"./xmodule/js/src/xmodule.js",
67+
"./xmodule/js/src/lti/lti.js"
68+
],
69+
"LTIBlockEditor": [
70+
"./xmodule/js/src/xmodule.js",
71+
"./xmodule/js/src/raw/edit/metadata-only.js"
72+
],
73+
"LibraryContentBlockDisplay": "./xmodule/js/src/xmodule.js",
74+
"LibraryContentBlockEditor": [
75+
"./xmodule/js/src/xmodule.js",
76+
"./xmodule/js/src/vertical/edit.js"
77+
],
78+
"PollBlockDisplay": [
79+
"./xmodule/js/src/xmodule.js",
80+
"./xmodule/js/src/javascript_loader.js",
81+
"./xmodule/js/src/poll/poll.js",
82+
"./xmodule/js/src/poll/poll_main.js"
83+
],
84+
"PollBlockEditor": "./xmodule/js/src/xmodule.js",
85+
"ProblemBlockDisplay": [
86+
"./xmodule/js/src/xmodule.js",
87+
"./xmodule/js/src/javascript_loader.js",
88+
"./xmodule/js/src/capa/display.js",
89+
"./xmodule/js/src/collapsible.js",
90+
"./xmodule/js/src/capa/imageinput.js",
91+
"./xmodule/js/src/capa/schematic.js"
92+
],
93+
"ProblemBlockEditor": [
94+
"./xmodule/js/src/xmodule.js",
95+
"./xmodule/js/src/problem/edit.js"
96+
],
97+
"SequenceBlockDisplay": [
98+
"./xmodule/js/src/xmodule.js",
99+
"./xmodule/js/src/sequence/display.js"
100+
],
101+
"SequenceBlockEditor": "./xmodule/js/src/xmodule.js",
102+
"SplitTestBlockDisplay": "./xmodule/js/src/xmodule.js",
103+
"SplitTestBlockEditor": [
104+
"./xmodule/js/src/xmodule.js",
105+
"./xmodule/js/src/sequence/edit.js"
106+
],
107+
"StaticTabBlockDisplay": [
108+
"./xmodule/js/src/xmodule.js",
109+
"./xmodule/js/src/html/display.js",
110+
"./xmodule/js/src/javascript_loader.js",
111+
"./xmodule/js/src/collapsible.js",
112+
"./xmodule/js/src/html/imageModal.js",
113+
"./xmodule/js/common_static/js/vendor/draggabilly.js"
114+
],
115+
"StaticTabBlockEditor": [
116+
"./xmodule/js/src/xmodule.js",
117+
"./xmodule/js/src/html/edit.js"
118+
],
119+
"VideoBlockDisplay": [
120+
"./xmodule/js/src/xmodule.js",
121+
"./xmodule/js/src/video/10_main.js"
122+
],
123+
"VideoBlockEditor": [
124+
"./xmodule/js/src/xmodule.js",
125+
"./xmodule/js/src/tabs/tabs-aggregator.js"
126+
],
127+
"WordCloudBlockDisplay": [
128+
"./xmodule/js/src/xmodule.js",
129+
"./xmodule/assets/word_cloud/src/js/word_cloud.js"
130+
],
131+
"WordCloudBlockEditor": [
132+
"./xmodule/js/src/xmodule.js",
133+
"./xmodule/js/src/raw/edit/metadata-only.js"
134+
]
135+
}
136+
};

‎webpack.common.config.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ var StringReplace = require('string-replace-webpack-plugin');
99
var Merge = require('webpack-merge');
1010

1111
var files = require('./webpack-config/file-lists.js');
12-
var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js');
12+
var builtinBlocksJS = require('./webpack.builtinblocks.config.js');
1313

1414
var filesWithRequireJSBlocks = [
1515
path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'),
@@ -553,4 +553,4 @@ module.exports = Merge.smart({
553553
}
554554

555555
}
556-
}, {web: xmoduleJS}, workerConfig());
556+
}, {web: builtinBlocksJS}, workerConfig());

‎xmodule/annotatable_block.py

-19
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import textwrap
55

66
from lxml import etree
7-
from pkg_resources import resource_filename
87
from web_fragments.fragment import Fragment
98
from xblock.core import XBlock
109
from xblock.fields import Scope, String
@@ -15,7 +14,6 @@
1514
from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment
1615
from xmodule.xml_block import XmlMixin
1716
from xmodule.x_module import (
18-
HTMLSnippet,
1917
ResourceTemplates,
2018
shim_xmodule_js,
2119
XModuleMixin,
@@ -35,7 +33,6 @@ class AnnotatableBlock(
3533
XmlMixin,
3634
EditingMixin,
3735
XModuleToXBlockMixin,
38-
HTMLSnippet,
3936
ResourceTemplates,
4037
XModuleMixin,
4138
):
@@ -73,22 +70,6 @@ class AnnotatableBlock(
7370

7471
uses_xmodule_styles_setup = True
7572

76-
preview_view_js = {
77-
'js': [
78-
resource_filename(__name__, 'js/src/html/display.js'),
79-
resource_filename(__name__, 'js/src/annotatable/display.js'),
80-
resource_filename(__name__, 'js/src/javascript_loader.js'),
81-
resource_filename(__name__, 'js/src/collapsible.js'),
82-
],
83-
'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'),
84-
}
85-
86-
studio_view_js = {
87-
'js': [
88-
resource_filename(__name__, 'js/src/raw/edit/xml.js'),
89-
],
90-
'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'),
91-
}
9273
studio_js_module_name = "XMLEditingDescriptor"
9374
mako_template = "widgets/raw-edit.html"
9475

‎xmodule/assets/README.rst

+5-8
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ the corresponding folders in any enabled themes, as part of the edx-platform bui
4141
It is collected into the static root, and then linked to from XBlock fragments by the
4242
``add_sass_to_fragment`` function in `builtin_assets.py`_.
4343

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

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

6060
* For many older blocks, their JS is:
6161

62-
* bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``,
62+
* bundled using a `webpack.builtinblocks.config.js`_,
6363
* which is included into `webpack.common.config.js`_,
6464
* allowing it to be included into XBlock fragments using ``add_webpack_js_to_fragment`` from `builtin_assets.py`_.
6565

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

77-
As part of an `active build refactoring`_:
78-
79-
* We will move ``webpack.xmodule.config.js`` here instead of generating it.
80-
* We will consolidate all edx-platform XBlock JS here in `xmodule/assets`_.
81-
* We will delete the ``xmodule_assets`` script.
77+
As part of an `active build refactoring`_, we will soon consolidate all edx-platform XBlock JS here in `xmodule/assets`_.
8278

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

0 commit comments

Comments
 (0)