From 8b3635ab7596a741f884841e9a8a6c45993ab569 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Fri, 3 Dec 2021 02:21:01 +0000 Subject: [PATCH] chore: Simplify NPM package wrappers, improve chunk wrapper generator (#5777) * chore: Clean up NPM package module wrappers - Slightly improve documentation for each file based on helpful explanations given by @samelhusseini. - Removed redundant code---e.g., loading `javascript_compressed.js` creates and sets Blockly.JavaScript as a side effect, so there is no need to set `Blockly.JavaScript = BlocklyJavaScript` in `dist/javascript.js` (generated from `scripts/package/javascript.js`). - Remove possibly harmful code---e.g., `Blockly.Msg` is initialised with a null-prototype object in `blockly_compressed.js` and that initial object should under no circumstances be replaced. - Remvoe downright misleading code---e.g., `dist/blocks.js` previously _appeared_ to replace Blockly.Blocks with an empty object, but in fact the `Blockly` name referred at that point to the exports object from `blocks_compressed.js`, which would randomly get a useless `{}`-valued `.Blocks` property tacked on to it; similarly, code in `dist/browser.js` (generated from `scripts/package/browser/index.js`) appeared to copy definitions from `BlocklyBlocks` to `Blockly.Blocks`, but the former would always be (the aforementioned) empty object, making this code ineffective. * chore: Improve chunk definition / UMD generation Make several improvements to the chunks global and chunkWrapper function: - Document chunk definition format (and improve the names of of the documented properties). - Replace the chunk `.namespace` property with two others: - `.exports` names the variable/property to be returned by the factory function, and which will be set on the global object if the module is loaded in a browser. - `.importAs` names the parameter that this chunk's exports value is to be passed to the factory function of other chunks which depend on this one. (This needs to be different because e.g. `Blockly.blocks` is not a valid parameter name.) - Change the definition for the blocks chunk to export Blockly.Blocks (i.e., the block definition dictionary) as blocks_compressed.js did previous to PR #5721 (chunked compilation), rather than the (empty and soon to vanish) Blockly.blocks namespace object. This is a win for backwards compatibility, though it does mean that if we want to expose the `loopTypes` export from `blocks/loops.js` we will need to find a different way to do so. --- scripts/gulpfiles/build_tasks.js | 62 ++++++++++++++++++++++-------- scripts/gulpfiles/package_tasks.js | 4 +- scripts/package/blockly.js | 5 +-- scripts/package/blocks.js | 7 +--- scripts/package/browser/core.js | 1 - scripts/package/browser/index.js | 7 ---- scripts/package/dart.js | 7 +--- scripts/package/index.js | 7 ++-- scripts/package/javascript.js | 8 +--- scripts/package/lua.js | 7 +--- scripts/package/node/core.js | 1 - scripts/package/node/index.js | 15 -------- scripts/package/php.js | 7 +--- scripts/package/python.js | 8 +--- 14 files changed, 60 insertions(+), 86 deletions(-) diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index b2abe9f98..aa7b77f39 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -66,7 +66,25 @@ const NAMESPACE_OBJECT = '$'; /** * A list of chunks. Order matters: later chunks can depend on * earlier ones, but not vice-versa. All chunks are assumed to depend - * on the first chunk. + * on the first chunk. Properties are as follows: + * + * - .name: the name of the chunk. Used to label it when describing + * it to Closure Compiler and forms the prefix of filename the chunk + * will be written to. + * - .entry: the source .js file which is the entrypoint for the + * chunk. + * - .exports: a variable or property that will (prefixed with + * NAMESPACE_OBJECT) be returned from the factory function and which + * (sans prefix) will be set in the global scope to that returned + * value if the module is loaded in a browser. + * - .importAs: the name that this chunk's exports object will be + * given when passed to the factory function of other chunks that + * depend on it. (Needs to be distinct from .exports since (e.g.) + * "Blockly.blocks.all" is not a valid variable name.) + * - .factoryPreamble: code to override the default wrapper factory + * function preamble. + * - .factoryPostamble: code to override the default wrapper factory + * function postabmle. * * The function getChunkOptions will, after running * closure-calculate-chunks, update each chunk to add the following @@ -81,37 +99,49 @@ const chunks = [ { name: 'blockly', entry: 'core/requires.js', - namespace: 'Blockly', - wrapperSetup: `const ${NAMESPACE_OBJECT}={};`, - wrapperCleanup: + exports: 'Blockly', + importAs: 'Blockly', + factoryPreamble: `const ${NAMESPACE_OBJECT}={};`, + factoryPostamble: `${NAMESPACE_OBJECT}.Blockly.internal_=${NAMESPACE_OBJECT};`, }, { name: 'blocks', entry: 'blocks/all.js', - namespace: 'Blockly.blocks', + exports: 'Blockly.Blocks', + importAs: 'BlocklyBlocks', }, { name: 'javascript', entry: 'generators/javascript/all.js', - namespace: 'Blockly.JavaScript', + exports: 'Blockly.JavaScript', }, { name: 'python', entry: 'generators/python/all.js', - namespace: 'Blockly.Python', + exports: 'Blockly.Python', }, { name: 'php', entry: 'generators/php/all.js', - namespace: 'Blockly.PHP', + exports: 'Blockly.PHP', }, { name: 'lua', entry: 'generators/lua/all.js', - namespace: 'Blockly.Lua', + exports: 'Blockly.Lua', }, { name: 'dart', entry: 'generators/dart/all.js', - namespace: 'Blockly.Dart', + exports: 'Blockly.Dart', } ]; +/** + * The default factory function premable. + */ +const FACTORY_PREAMBLE = `const ${NAMESPACE_OBJECT}=Blockly.internal_;`; + +/** + * The default factory function postamble. + */ +const FACTORY_POSTAMBLE = ''; + const licenseRegex = `\\/\\*\\* \\* @license \\* (Copyright \\d+ (Google LLC|Massachusetts Institute of Technology)) @@ -283,8 +313,8 @@ function chunkWrapper(chunk) { const amdDeps = fileNames.join(', '); const cjsDeps = fileNames.map(f => `require(${f})`).join(', '); const browserDeps = - chunk.dependencies.map(d => `root.${d.namespace}`).join(', '); - const imports = chunk.dependencies.map(d => d.namespace).join(', '); + chunk.dependencies.map(d => `root.${d.exports}`).join(', '); + const imports = chunk.dependencies.map(d => d.importAs).join(', '); return `// Do not edit this file; automatically generated. /* eslint-disable */ @@ -294,13 +324,13 @@ function chunkWrapper(chunk) { } else if (typeof exports === 'object') { // Node.js module.exports = factory(${cjsDeps}); } else { // Browser - root.${chunk.namespace} = factory(${browserDeps}); + root.${chunk.exports} = factory(${browserDeps}); } }(this, function(${imports}) { -${chunk.wrapperSetup || `const ${NAMESPACE_OBJECT}=Blockly.internal_;`} +${chunk.factoryPreamble || FACTORY_PREAMBLE} %output% -${chunk.wrapperCleanup || ''} -return ${NAMESPACE_OBJECT}.${chunk.namespace}; +${chunk.factoryPostamble || FACTORY_POSTAMBLE} +return ${NAMESPACE_OBJECT}.${chunk.exports}; })); `; }; diff --git a/scripts/gulpfiles/package_tasks.js b/scripts/gulpfiles/package_tasks.js index fefb2d9f8..9d7edca04 100644 --- a/scripts/gulpfiles/package_tasks.js +++ b/scripts/gulpfiles/package_tasks.js @@ -126,8 +126,8 @@ function packageBlockly() { */ function packageBlocks() { return gulp.src('scripts/package/blocks.js') - .pipe(packageUMD('Blockly.Blocks', [{ - name: 'Blockly', + .pipe(packageUMD('BlocklyBlocks', [{ + name: 'BlocklyBlocks', amd: './blocks_compressed', cjs: './blocks_compressed', }])) diff --git a/scripts/package/blockly.js b/scripts/package/blockly.js index 55bc791af..77021b3d6 100644 --- a/scripts/package/blockly.js +++ b/scripts/package/blockly.js @@ -5,8 +5,5 @@ */ /** - * @fileoverview Blockly module. + * @fileoverview Blockly module; just a wrapper for blockly_compressed.js. */ - -/* eslint-disable */ -'use strict'; diff --git a/scripts/package/blocks.js b/scripts/package/blocks.js index 8e0d4ab79..66ae806bf 100644 --- a/scripts/package/blocks.js +++ b/scripts/package/blocks.js @@ -5,10 +5,5 @@ */ /** - * @fileoverview Blockly Blocks module. + * @fileoverview Blockly blocks module; just a wrapper for blocks_compressed.js. */ - -/* eslint-disable */ -'use strict'; - -Blockly.Blocks = {}; diff --git a/scripts/package/browser/core.js b/scripts/package/browser/core.js index 19463cd7e..a0b0d9259 100644 --- a/scripts/package/browser/core.js +++ b/scripts/package/browser/core.js @@ -14,7 +14,6 @@ // Add a helper method to set the Blockly locale. Blockly.setLocale = function (locale) { - Blockly.Msg = Blockly.Msg || {}; Object.keys(locale).forEach(function (k) { Blockly.Msg[k] = locale[k]; }); diff --git a/scripts/package/browser/index.js b/scripts/package/browser/index.js index 1b2f05e51..36d0c325b 100644 --- a/scripts/package/browser/index.js +++ b/scripts/package/browser/index.js @@ -15,10 +15,3 @@ // Include the EN Locale by default. Blockly.setLocale(En); - -Blockly.Blocks = Blockly.Blocks || {}; -Object.keys(BlocklyBlocks).forEach(function (k) { - Blockly.Blocks[k] = BlocklyBlocks[k]; -}); - -Blockly.JavaScript = BlocklyJS; \ No newline at end of file diff --git a/scripts/package/dart.js b/scripts/package/dart.js index ea4e8338a..d59060bde 100644 --- a/scripts/package/dart.js +++ b/scripts/package/dart.js @@ -5,10 +5,5 @@ */ /** - * @fileoverview Dart Generator module. + * @fileoverview Dart generator module; just a wrapper for dart_compressed.js. */ - -/* eslint-disable */ -'use strict'; - -Blockly.Dart = BlocklyDart; diff --git a/scripts/package/index.js b/scripts/package/index.js index f88c16e73..0264fd446 100644 --- a/scripts/package/index.js +++ b/scripts/package/index.js @@ -5,8 +5,7 @@ */ /** - * @fileoverview Blockly module. + * @fileoverview Blockly module; this is a wrapper which selects + * either browser.js or node.js, depending on which environment we + * are running in. */ - -/* eslint-disable */ -'use strict'; \ No newline at end of file diff --git a/scripts/package/javascript.js b/scripts/package/javascript.js index e0ba813f2..e4adba09f 100644 --- a/scripts/package/javascript.js +++ b/scripts/package/javascript.js @@ -5,10 +5,6 @@ */ /** - * @fileoverview JavaScript Generator module. + * @fileoverview JavaScript Generator module; just a wrapper for + * javascript_compressed.js. */ - -/* eslint-disable */ -'use strict'; - -Blockly.JavaScript = BlocklyJavaScript; diff --git a/scripts/package/lua.js b/scripts/package/lua.js index 4094782a1..ca37b44f9 100644 --- a/scripts/package/lua.js +++ b/scripts/package/lua.js @@ -5,10 +5,5 @@ */ /** - * @fileoverview Lua Generator module. + * @fileoverview Lua generator module; just a wrapper for lua_compressed.js. */ - -/* eslint-disable */ -'use strict'; - -Blockly.Lua = BlocklyLua; diff --git a/scripts/package/node/core.js b/scripts/package/node/core.js index 9692855d3..36a5addd2 100644 --- a/scripts/package/node/core.js +++ b/scripts/package/node/core.js @@ -14,7 +14,6 @@ // Add a helper method to set the Blockly locale. Blockly.setLocale = function (locale) { - Blockly.Msg = Blockly.Msg || {}; Object.keys(locale).forEach(function (k) { Blockly.Msg[k] = locale[k]; }); diff --git a/scripts/package/node/index.js b/scripts/package/node/index.js index d50a92d60..33c322a98 100644 --- a/scripts/package/node/index.js +++ b/scripts/package/node/index.js @@ -14,18 +14,3 @@ // Include the EN Locale by default. Blockly.setLocale(En); - -Blockly.Blocks = Blockly.Blocks || {}; -Object.keys(BlocklyBlocks).forEach(function (k) { - Blockly.Blocks[k] = BlocklyBlocks[k]; -}); - -Blockly.JavaScript = BlocklyJS; - -Blockly.Python = BlocklyPython; - -Blockly.Lua = BlocklyLua; - -Blockly.PHP = BlocklyPHP; - -Blockly.Dart = BlocklyDart; \ No newline at end of file diff --git a/scripts/package/php.js b/scripts/package/php.js index 361dd97ec..a6ed3fa6d 100644 --- a/scripts/package/php.js +++ b/scripts/package/php.js @@ -5,10 +5,5 @@ */ /** - * @fileoverview PHP Generator module. + * @fileoverview PHP generator module; just a wrapper for php_compressed.js. */ - -/* eslint-disable */ -'use strict'; - -Blockly.PHP = BlocklyPHP; diff --git a/scripts/package/python.js b/scripts/package/python.js index 3634e41bf..0bb64c06f 100644 --- a/scripts/package/python.js +++ b/scripts/package/python.js @@ -5,10 +5,6 @@ */ /** - * @fileoverview Python Generator module. + * @fileoverview Python generator module; just a wrapper for + * python_compressed.js. */ - -/* eslint-disable */ -'use strict'; - -Blockly.Python = BlocklyPython;