From d629bc52561aa69c36eb3dfc42a1a8fcd2447ab1 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 29 Jun 2022 19:22:55 +0100 Subject: [PATCH 1/9] fix(build): Minor corrections to build_tasks.js - Use TSC_OUTPUT_DIR to find goog/goog.js when suppressing warnings. - Remove unnecessary trailing semicolons. --- scripts/gulpfiles/build_tasks.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index 3e709f940cf..0b79dba15fc 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -311,7 +311,7 @@ function buildDeps(done) { execSync(`closure-make-deps ${testArgs} | grep 'tests/mocha' ` + `> '${TEST_DEPS_FILE}'`, {stdio: 'inherit'}); done(); -}; +} /** * This task regenrates msg/json/en.js and msg/json/qqq.js from @@ -341,7 +341,7 @@ this removal! `); done(); -}; +} /** * This task builds Blockly's lang files. @@ -367,7 +367,7 @@ function buildLangfiles(done) { execSync(createMessagesCmd, {stdio: 'inherit'}); done(); -}; +} /** * A helper method to return an closure compiler chunk wrapper that @@ -434,7 +434,7 @@ ${exportsExpression}.${NAMESPACE_PROPERTY}=${NAMESPACE_VARIABLE}; return ${exportsExpression}; })); `; -}; +} /** * Get chunking options to pass to Closure Compiler by using @@ -566,7 +566,10 @@ function compile(options) { // declared by base_minimal.js, while if you compile against // base.js instead you will discover that it uses @deprecated // inherits, forwardDeclare etc. - hide_warnings_for: ['node_modules', 'build/src/closure/goog/goog.js'], + hide_warnings_for: [ + 'node_modules', + path.join(TSC_OUTPUT_DIR, 'closure', 'goog', 'goog.js'), + ], define: ['COMPILED=true'], }; if (argv.debug || argv.strict) { @@ -611,7 +614,7 @@ function buildCompiled() { .pipe( gulp.sourcemaps.write('.', {includeContent: false, sourceRoot: './'})) .pipe(gulp.dest(BUILD_DIR)); -}; +} /** * This task builds Blockly core, blocks and generators together and uses @@ -671,7 +674,7 @@ function checkinBuilt() { `${BUILD_DIR}/*_compressed.js.map`, `${BUILD_DIR}/msg/js/*.js`, ], {base: BUILD_DIR}).pipe(gulp.dest('.')); -}; +} /** * This task cleans the build directory (by deleting it). @@ -694,7 +697,7 @@ function format() { ], {base: '.'}) .pipe(clangFormatter.format('file', clangFormat)) .pipe(gulp.dest('.')); -}; +} module.exports = { build: build, From 34afb4227df91c5c38a4eebd6fa6bffead16d8d3 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 29 Jun 2022 19:27:16 +0100 Subject: [PATCH 2/9] refactor(blocks): Remove declareLegacyNamespace Remove the call to goog.module.declareLegacyNamespace from Blockly.libraryBlocks. This entails: - Changes to the UMD wrapper to be able to find the exports object. - Changes to tests/bootstrap_helper.js to save the exports object in the libraryBlocks global variable. - As a precaution, renaming the tests/compile/test_blocks.js module so that goog.provide does not touch Blockly or Blockly.libraryBlocks, which may not exist / be writable. --- blocks/blocks.js | 1 - scripts/gulpfiles/build_tasks.js | 16 +++++++--------- tests/bootstrap_helper.js | 7 +++++++ tests/compile/main.js | 2 +- tests/compile/test_blocks.js | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/blocks/blocks.js b/blocks/blocks.js index f6758c2c51b..57d72f59882 100644 --- a/blocks/blocks.js +++ b/blocks/blocks.js @@ -11,7 +11,6 @@ 'use strict'; goog.module('Blockly.libraryBlocks'); -goog.module.declareLegacyNamespace(); const colour = goog.require('Blockly.libraryBlocks.colour'); const lists = goog.require('Blockly.libraryBlocks.lists'); diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index 0b79dba15fc..1a4055f1489 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -87,6 +87,9 @@ const NAMESPACE_PROPERTY = '__namespace__'; * will be written to. * - .entry: the source .js file which is the entrypoint for the * chunk. + * - .exports: an expression evaluating to the exports/Module object + * of module that is the chunk's entrypoint / top level module. + * Will guess based on .reexport if not supplied. * - .reexport: if running in a browser, save the chunk's exports * object at this location in the global namespace. * @@ -109,6 +112,7 @@ const chunks = [ { name: 'blocks', entry: 'blocks/blocks.js', + exports: 'module$exports$Blockly$libraryBlocks', reexport: 'Blockly.libraryBlocks', }, { @@ -400,15 +404,9 @@ function chunkWrapper(chunk) { } // Expression that evaluates the the value of the exports object for - // the specified chunk. For now we guess the name that is created - // by the module's goog.module.delcareLegacyNamespace call based on - // chunk.reexport. - const exportsExpression = `${NAMESPACE_VARIABLE}.${chunk.reexport}`; - // In near future we might try to guess the internally-generated - // name for the ES module's exports object. - // const exportsExpression = - // 'module$' + chunk.entry.replace(/\.m?js$/, '').replace(/\//g, '$'); - + // the specified chunk. + const exportsExpression = + chunk.exports || `${NAMESPACE_VARIABLE}.${chunk.reexport}`; // Note that when loading in a browser the base of the exported path // (e.g. Blockly.blocks.all - see issue #5932) might not exist diff --git a/tests/bootstrap_helper.js b/tests/bootstrap_helper.js index ba61ff2eab4..c450c9d32d7 100644 --- a/tests/bootstrap_helper.js +++ b/tests/bootstrap_helper.js @@ -17,4 +17,11 @@ /* eslint-disable-next-line no-undef */ for (const require of window.bootstrapInfo.requires) { goog.require(require); + + // If require is a top-level chunk, create a global variable for it. + // This replaces the goog.module.declareLegacyNamespace calls that + // previously existed in each chunk entrypoint. + if (require === 'Blockly.libraryBlocks') { + window.libraryBlocks = goog.module.get(require); + } } diff --git a/tests/compile/main.js b/tests/compile/main.js index b5d20a770cd..644befef384 100644 --- a/tests/compile/main.js +++ b/tests/compile/main.js @@ -25,7 +25,7 @@ goog.require('Blockly.libraryBlocks.math'); /** @suppress {extraRequire} */ goog.require('Blockly.libraryBlocks.texts'); /** @suppress {extraRequire} */ -goog.require('Blockly.libraryBlocks.testBlocks'); +goog.require('testBlocks'); function init() { diff --git a/tests/compile/test_blocks.js b/tests/compile/test_blocks.js index 2b8cc6621bc..7de1b45241a 100644 --- a/tests/compile/test_blocks.js +++ b/tests/compile/test_blocks.js @@ -9,7 +9,7 @@ */ 'use strict'; -goog.provide('Blockly.libraryBlocks.testBlocks'); +goog.provide('testBlocks'); goog.require('Blockly'); From b936473e6dc2a0a206e2cc9802738dae8d93c790 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 29 Jun 2022 22:11:32 +0100 Subject: [PATCH 3/9] feat(build): Add support named exports from chunks We need to convert the generators to named exports. For backwards compatibility we still want e.g. Blockly.JavaScript to point at the generator object when the chunk is loaded using a script tag. Modify chunkWrapper to honour a .reexportOnly property in the chunks table and generate suitable additional code in the UMD wrapper. --- scripts/gulpfiles/build_tasks.js | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index 1a4055f1489..e0c185b8632 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -91,7 +91,11 @@ const NAMESPACE_PROPERTY = '__namespace__'; * of module that is the chunk's entrypoint / top level module. * Will guess based on .reexport if not supplied. * - .reexport: if running in a browser, save the chunk's exports - * object at this location in the global namespace. + * object (or a single export of it; see reexportOnly, below) at + * this location in the global namespace. + * - .reexportOnly: if reexporting and this property is set, + * save only the correspondingly-named export. Otherwise + * save the whole export object. * * The function getChunkOptions will, after running * closure-calculate-chunks, update each chunk to add the following @@ -408,6 +412,21 @@ function chunkWrapper(chunk) { const exportsExpression = chunk.exports || `${NAMESPACE_VARIABLE}.${chunk.reexport}`; + // Code to assign the result of the factory function to the desired + // export location when running in a browser. When + // chunk.reexportOnly is set, this additionally does two other + // things: + // - It ensures that only the desired property of the exports object + // is assigned to the specified reexport location. + // - It ensures that the namesspace object is accessible via the + // selected sub-object, so that any dependent modules can obtain + // it. + const browserExportStatements = chunk.reexportOnly ? + `root.${chunk.reexport} = factoryExports.${chunk.reexportOnly};\n` + + ` root.${chunk.reexport}.${NAMESPACE_PROPERTY} = ` + + `factoryExports.${NAMESPACE_PROPERTY};` : + `root.${chunk.reexport} = factoryExports;`; + // Note that when loading in a browser the base of the exported path // (e.g. Blockly.blocks.all - see issue #5932) might not exist // before factory has been executed, so calling factory() and @@ -423,7 +442,7 @@ function chunkWrapper(chunk) { module.exports = factory(${cjsDepsExpr}); } else { // Browser var factoryExports = factory(${browserDepsExpr}); - root.${chunk.reexport} = factoryExports; + ${browserExportStatements} } }(this, function(${factoryArgs}) { var ${NAMESPACE_VARIABLE}=${namespaceExpr}; From 9c761ecd53c313eb45f9717784553b45663148cd Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 29 Jun 2022 23:55:18 +0100 Subject: [PATCH 4/9] refactor(generators): Migrate JavaScript generator to named export - Export the JavaScript generator object as javascriptGenerator from the Blockly.JavaScript module(generators/javascript.js). - Modify the Blockly.JavaScript.all module (generators/javascript/all.js) to reexport the exports from Blockly.JavaScript. - Update chunk configuration so the generator object remains available as Blockly.JavaScript when loading javascript_compressed.js via a