From cbff33a0427ce715425952446b232369386b10d5 Mon Sep 17 00:00:00 2001 From: Irfan Ahmad Date: Fri, 5 Jun 2026 15:29:32 +0500 Subject: [PATCH 1/4] feat: modernize Problem Block JS, enable Babel on capa files Removes the broad /xmodule\/js\/src/ regex from the string-replace-loader and babel-loader exclusion lists, replacing it with explicit paths for only the three files that actually contain RequireJS patterns (poll.js, poll_main.js, 08_video_auto_advance_control.js). This allows Babel to process the capa files (display.js, imageinput.js, schematic.js). Also fixes an implicit global variable in schematic.js that would throw under Babel strict mode, and migrates capa Karma tests from the legacy runner to the webpack-based runner. Closes https://github.com/openedx/public-engineering/issues/438 Co-Authored-By: Claude Sonnet 4.6 --- webpack-config/file-lists.js | 4 +++- webpack.common.config.js | 4 +++- xmodule/js/karma_runner_webpack.js | 12 ++++++++++++ xmodule/js/karma_xmodule.conf.js | 4 ++-- xmodule/js/src/capa/schematic.js | 2 +- 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/webpack-config/file-lists.js b/webpack-config/file-lists.js index f92664881395..cf0a64eaaa63 100644 --- a/webpack-config/file-lists.js +++ b/webpack-config/file-lists.js @@ -10,7 +10,9 @@ 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'), - /xmodule\/js\/src/, + path.resolve(__dirname, '../xmodule/js/src/poll/poll.js'), + path.resolve(__dirname, '../xmodule/js/src/poll/poll_main.js'), + path.resolve(__dirname, '../xmodule/js/src/video/08_video_auto_advance_control.js'), path.resolve(__dirname, '../openedx/features/course_bookmarks/static/course_bookmarks/js/views/bookmark_button.js') ], diff --git a/webpack.common.config.js b/webpack.common.config.js index cd9d0f53af59..0bab7c9a02b4 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -12,7 +12,9 @@ var builtinBlocksJS = require('./webpack.builtinblocks.config.js'); var filesWithRequireJSBlocks = [ path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'), - /xmodule\/js\/src/ + path.resolve(__dirname, 'xmodule/js/src/poll/poll.js'), + path.resolve(__dirname, 'xmodule/js/src/poll/poll_main.js'), + path.resolve(__dirname, 'xmodule/js/src/video/08_video_auto_advance_control.js'), ]; var defineHeader = /\(function ?\(((define|require|requirejs|\$)(, )?)+\) ?\{/; diff --git a/xmodule/js/karma_runner_webpack.js b/xmodule/js/karma_runner_webpack.js index d00be7111757..42feafbc28db 100644 --- a/xmodule/js/karma_runner_webpack.js +++ b/xmodule/js/karma_runner_webpack.js @@ -35,11 +35,23 @@ import StringUtils from 'edx-ui-toolkit/js/utils/string-utils'; // but not explicitly imported import 'jquery.ui'; +// Problem Block (capa) source files +import './src/xmodule.js'; +import './src/javascript_loader.js'; +import './src/collapsible.js'; +import './src/capa/display.js'; +import './src/capa/imageinput.js'; +import './src/capa/schematic.js'; + // These import '../assets/video/public/js/10_main.js'; import './spec/helper.js'; import './spec/video_helper.js'; +// Problem Block (capa) spec files +import './spec/capa/display_spec.js'; +import './spec/capa/imageinput_spec.js'; + // These are the tests that will be run import './spec/video/async_process_spec.js'; import './spec/video/completion_spec.js'; diff --git a/xmodule/js/karma_xmodule.conf.js b/xmodule/js/karma_xmodule.conf.js index 185e1e66883a..2cc04a7f1eec 100644 --- a/xmodule/js/karma_xmodule.conf.js +++ b/xmodule/js/karma_xmodule.conf.js @@ -74,12 +74,12 @@ var options = { {pattern: 'src/javascript_loader.js', included: true}, {pattern: 'src/collapsible.js', included: true}, // Load everything else - {pattern: 'src/**/!(video)/!(poll|time).js', included: true} + {pattern: 'src/**/!(video|capa)/!(poll|time).js', included: true} ], specFiles: [ {pattern: 'spec/helper.js', included: true, ignoreCoverage: true}, // Helper which depends on source files. - {pattern: 'spec/**/!(video)/*.js', included: true}, + {pattern: 'spec/**/!(video|capa)/*.js', included: true}, {pattern: 'spec/!(time_spec|video_helper).js', included: true} ], diff --git a/xmodule/js/src/capa/schematic.js b/xmodule/js/src/capa/schematic.js index 7004562af8ce..0babbdae525b 100644 --- a/xmodule/js/src/capa/schematic.js +++ b/xmodule/js/src/capa/schematic.js @@ -1978,7 +1978,7 @@ function update_schematics() { } window.update_schematics = update_schematics; -schematic = (function () { +var schematic = (function () { var background_style = "rgb(220,220,220)"; var element_style = "rgb(255,255,255)"; var thumb_style = "rgb(128,128,128)"; From e3f46f3a0e0690daa2501cea7533d454555789c8 Mon Sep 17 00:00:00 2001 From: Irfan Ahmad Date: Fri, 5 Jun 2026 15:45:10 +0500 Subject: [PATCH 2/4] fix: set window.imagediff for jasmine-extensions matcher registration jasmine-extensions.js registers toImageDiffEqual only if window.imagediff is set. When imported via webpack, jasmine-imagediff.js uses the CommonJS path (module.exports) and never sets window.imagediff. Import it as a named default and assign to window explicitly so the matcher is available for display_spec.js imageinput tests. Co-Authored-By: Claude Sonnet 4.6 --- xmodule/js/karma_runner_webpack.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xmodule/js/karma_runner_webpack.js b/xmodule/js/karma_runner_webpack.js index 42feafbc28db..e82bb1fc75ea 100644 --- a/xmodule/js/karma_runner_webpack.js +++ b/xmodule/js/karma_runner_webpack.js @@ -22,7 +22,8 @@ import '../../common/static/js/test/i18n.js'; import '../../common/static/common/js/vendor/hls.js'; import '../assets/vertical/public/js/vertical_student_view.js'; -import '../../common/static/js/vendor/jasmine-imagediff.js'; +import imagediff from '../../common/static/js/vendor/jasmine-imagediff.js'; +window.imagediff = imagediff; import '../../common/static/common/js/spec_helpers/jasmine-waituntil.js'; import '../../common/static/common/js/spec_helpers/jasmine-extensions.js'; import '../../common/static/common/js/vendor/sinon.js'; From 82a428afaacc53fa53554c18cdd73a2e035a2491 Mon Sep 17 00:00:00 2001 From: Irfan Ahmad Date: Fri, 5 Jun 2026 15:49:48 +0500 Subject: [PATCH 3/4] fix: move window.imagediff assignment after all imports ES module imports are hoisted, so the assignment must live after all imports (alongside other window.* globals). This is safe because jasmine-extensions.js registers the addMatchers call inside a beforeEach callback, so window.imagediff only needs to be set before tests run, not at module load time. Co-Authored-By: Claude Sonnet 4.6 --- xmodule/js/karma_runner_webpack.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/js/karma_runner_webpack.js b/xmodule/js/karma_runner_webpack.js index e82bb1fc75ea..d276e48cca96 100644 --- a/xmodule/js/karma_runner_webpack.js +++ b/xmodule/js/karma_runner_webpack.js @@ -23,7 +23,6 @@ import '../../common/static/common/js/vendor/hls.js'; import '../assets/vertical/public/js/vertical_student_view.js'; import imagediff from '../../common/static/js/vendor/jasmine-imagediff.js'; -window.imagediff = imagediff; import '../../common/static/common/js/spec_helpers/jasmine-waituntil.js'; import '../../common/static/common/js/spec_helpers/jasmine-extensions.js'; import '../../common/static/common/js/vendor/sinon.js'; @@ -96,6 +95,7 @@ import './spec/time_spec.js'; 'use strict'; window._ = _; +window.imagediff = imagediff; window.edx = window.edx || {}; window.edx.HtmlUtils = HtmlUtils; window.edx.StringUtils = StringUtils; From b169f10f18d1c95fdc3430f209b1443ddc53adbf Mon Sep 17 00:00:00 2001 From: Irfan Ahmad Date: Fri, 5 Jun 2026 16:15:07 +0500 Subject: [PATCH 4/4] docs: add comments explaining explicit RequireJS file lists Clarifies why the three xmodule/js/src/ files are listed explicitly and signals to future contributors that new files should use ES6+ and must not be added here. Co-Authored-By: Claude Sonnet 4.6 --- webpack-config/file-lists.js | 3 +++ webpack.common.config.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/webpack-config/file-lists.js b/webpack-config/file-lists.js index cf0a64eaaa63..6e07782ae94a 100644 --- a/webpack-config/file-lists.js +++ b/webpack-config/file-lists.js @@ -10,6 +10,9 @@ 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'), + // Files in xmodule/js/src/ that still use RequireJS (AMD) patterns. + // New files added under xmodule/js/src/ should use ES6+ — do NOT add them here. + // To migrate a file off RequireJS, remove it from this list. path.resolve(__dirname, '../xmodule/js/src/poll/poll.js'), path.resolve(__dirname, '../xmodule/js/src/poll/poll_main.js'), path.resolve(__dirname, '../xmodule/js/src/video/08_video_auto_advance_control.js'), diff --git a/webpack.common.config.js b/webpack.common.config.js index 0bab7c9a02b4..5fccaf6ff054 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -12,6 +12,9 @@ var builtinBlocksJS = require('./webpack.builtinblocks.config.js'); var filesWithRequireJSBlocks = [ path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'), + // Files in xmodule/js/src/ that still use RequireJS (AMD) patterns. + // New files added under xmodule/js/src/ should use ES6+ — do NOT add them here. + // To migrate a file off RequireJS, remove it from this list. path.resolve(__dirname, 'xmodule/js/src/poll/poll.js'), path.resolve(__dirname, 'xmodule/js/src/poll/poll_main.js'), path.resolve(__dirname, 'xmodule/js/src/video/08_video_auto_advance_control.js'),