chore(generators): Migrate generators to ES Modules #7103
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Convert
const {foo: bar} = require(/*...*/);
into
import {foo as bar} from /*...*/;
^^^^^^^^^^
Also fix a bug that caused relative paths to ESM in the same
directory to be missing a leading "./".
The UMD wrapper was inadvertently exporting the contents of (e.g.) the Blockly.JavaScript closure module rather than the intended export of Blockly.JavaScript.all module - which went unnoticed because the latter just reexported the former - but we are about to convert the former to ESM.
Migrate the main language generators in generators/*.js to ESM. This was done by running js2ts on the files, renaming them back to .js, and commenting out "import type" statements, which are legal TS but not needed in JS (at least if you are not actually letting Closure Compiler do type checking, which we are not.)
Migrate generators/*/*.js (except all.js) to ESM. This was done by running js2ts on the files, renaming them back to .js, and removing now-spurious @Suppress {extraRequire} directives.
This was done by running js2ts on the files, renaming them back to .js, and manually fixing the export statements. An additional change to the chunk exports configuration in build_tasks.js was necessary in order for the UMD wrapper to find the new module object, which is given a different name than the old exports object.
cpcallen
added
component: generators
PR: chore
github-actions
bot
added
PR: chore
BeksOmega
approved these changes
May 19, 2023
|
@maribethb asked if this will change anything about how devs import generators. |
No, absolutely not as far as I am aware. The proposed changes to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The basics
npm run formatandnpm run lintgenerators/.)The details
Resolves
Part of #6828 and #6858.
Proposed Changes
Add support for migrating renaming imports to
js2ts: convertinto
Also fix a bug that caused relative paths to ESM in the same directory to be missing a leading
./.Migrate all the
goog.modulemodules ingenerators/(that is: all.jsfiles in that subtree) to ES modules, replacinggoog.requirewithimport.js2tsthen renaming the.tsfile back to.js, with some small manual fixup.tscdoes not permitimport typestatements in.jsfiles, such statements generated byjs2tsare commented out for now.Behaviour Before Change
These modules are loaded as
goog.modules.Behaviour After Change
These modules are loaded as ES modules.
Reason for Changes
This is the first step of migrating these files to TypeScript, and also the only part of the TypeScript migration required in order to complete (their part) of the work to remove dependencies on the Closure module system (except for
goog.declareModuleIdcalls, which are still required due to a couple of the tests still usinggoog.require).Test Coverage
Passes
npm testand quick manual check that generators are not completely broken in the playground. No changes to manual testing anticipated.Documentation
Should be (approximately) no visible external changes.
Additional Information
As usual with such migrations there are some changes to the internal names generated by Closure Compiler. This necessitated some small changes to the chunk configuration in
build_tasks.jsand will break devs who monkey patchnon-public generator methods.
There are some additional details discussed in the individual commit messages.