-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preparation for goog.module transition: base.js, deps.js #5019
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
The version of base.js we have been using is a cut-down copy of an older version, stripped of everything that wasn't strictly needed for Blockly. We now need things that were previously stripped out, so restore the current version from https://github.com/google/closure-library/blob/master/closure/goog/base.js as of commit 5cb5e81 (2021-06-04). We can always strip out unused sections again in future - once we know which sections those are.
* New gulp task buildDeps (npm run build:deps) to create tests/deps.js. * Old gulp task buildUncompressed is deleted. * blockly_uncompressed.js is now handwritten. * And simplified; in particular, BLOCKLY_BOOT is gone. * And linted! * For consistency with the Closure documentation and base.js, consistently refer to what blockly_uncompressed.js is used for as "uncompiled mode" rather than "uncompressed mode" (but don't yet rename it to blockly_uncompiled.js)
cpcallen
force-pushed
the
goog.module-prep
branch
from
July 12, 2021 01:28
c50f4bb
to
f5a9f2c
Compare
Modules defined using goog.module declare their exported functions by assigning them to properties on an object which is the value of the variable "exports" (e.g., "exports.MyClass = class { ... };"). Normally eslint would complain about this variable being undefined, but this commit suppresses these errors.
Some type annotations were missing curly brackets, which makes closure-make-deps emit uninteresting warnings. Now any output from the command will be informative and related to whatever one is presently working on.
eslint appears to be confused by the leading JSDoc comment on the continuation line and suggests it should be indented only 4 rather than 8 spaces.
rachel-fenichel
approved these changes
Jul 13, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved after lint fix.
For whatever reason eslint was rejecting the (styleguide-correct) indentation of the line beginning with "/** @type ...", and in my naïvete I got a little more zealous than I intended in suppressing the error.
|
12 tasks
This resolves a conflict in `blockly_uncompressed.js`, and missing updates to `test/deps.js`, caused by PR google#5041.
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
[ ] I branched from develop[ ] My pull request is against developThe details
Proposed Changes
closure/goog/base.js
.tests/deps.js
.buildDeps
(npm run build:deps
) to createtests/deps.js
.blockly_uncompressed.js
is now handwritten.buildUncompressed
is deleted.eslint
ignore "undefined" global variableexports
, used to specify exports fromgoog.module
modules.Reason for Changes
The version of base.js we have been using is a cut-down copy of an older version, stripped of everything that wasn't strictly needed for Blockly. We now need things that were previously stripped out, so restore the current version from https://github.com/google/closure-library/blob/master/closure/goog/base.js. We can always strip out unused sections again in future - once we know which sections those are.
Loading dependency graph info from
tests/deps.js
(instead of having it embedded inblockly_uncompressed.js
helps maintain a clear separation between generated and handwritten code, avoids having code duplicated between the generator script and its output, and helped find some actual errors the handwritten parts because they can now be linted. It also more closely follows the Closure Library convention.Additional Information
This PR (or another to the same effect) must be merged to the
goog_module
branch before any others that actually carrying outgoog.provides
->goog.module
migration.