Skip to content
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

refactor(build): Prepare UMD wrapper generation for transition to ES modules #5993

Merged
merged 6 commits into from
Mar 21, 2022

Conversation

cpcallen
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Part of #5904.

Proposed Changes

Makes various changes to the way UMD chunk wrappers are generated. Specifically:

  • Corrects erroneous assumption that closure-calculate-chunks outputs chunks in the same order as the --entrypoints were specified on the command line.
  • Introduces the configuration constant NAMESPACE_PROPERTY to control where the namespace object is saved on the exports object.
  • Save the namespace object on every chunk exports object.
  • Corrects erroneous assumption that chunks could have multiple dependencies; instead, they have a single parent chunk.
  • Miscellaneous other updates to chunk wrapper generation.

Behaviour Before Change

The namespace object is saved only on the .internal_ property of the first (blockly) chunk's exports object.

Behaviour After Change

The namespace object is saved on the .__namespace__ property of every chunk's exports object.

Reason for Changes

  • Correction of erroneous code so as to avoid generating incorrect output if erroneous assumptions are violated in future.
  • Refactor chunk wrapper generation to make it easier to modify to support ES modules, which will not have goog.module.declareLegacyNamespace calls.

Test Coverage

  • Passes npm test.

Documentation

Additional Information

It turns out that closure-calculate-chunks does not guarantee that
calculated chunks will be output in the same order as the
entrypoints, so modify getChunkOptions so that it no longer makes
that assumption.
Rename the constant NAMESPACE_OBJECT to NAMESPACE_VARIABLE to
better explain its actual meaning, and introduce
NAMESPACE_PROPERTY to specify what property the namespace object
will be stored in (and change the previous value, "internal_",
to "__namespace__" to reduce the chance of conflicts with
properties created by the output of Closure Compiler).
This is so that chunks whose parent chunk is not the root chunk
(chunks[0]) can obtain the namespace object.  (See following commit.)
Previously getChunkOptions and chunkWrapper incorrectly assumed
that a chunk could have more than one dependency.

In fact, each chunk can have only a single dependency, which is
its parent chunk.  It is used only to retrieve the namespace
object, which is saved on to the exports object for the chunk so
that any child chunk(s) can obtain it.

Update getChunkOptions and chunkWrapper (making the latter
longer but more readable) accordingly.
And remove chunk.importAs, since it was no longer being used
anywhere.
@cpcallen cpcallen requested a review from gonfunko March 11, 2022 22:22
@cpcallen cpcallen marked this pull request as ready for review March 18, 2022 20:32
@cpcallen cpcallen requested a review from a team as a code owner March 18, 2022 20:32
@cpcallen
Copy link
Contributor Author

@gonfunko / @alschmiedt: please feel free to apply any suggested changes directly, and then go ahead and merge.

@alschmiedt alschmiedt merged commit 9d62f92 into google:develop Mar 21, 2022
@cpcallen cpcallen deleted the better-wrappers branch June 21, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants