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

Rename module identifier by wrapping program.body with IIFE. #229

Merged
merged 3 commits into from
Jun 18, 2019

Conversation

benjamn
Copy link
Owner

@benjamn benjamn commented Jun 18, 2019

Before this change, in order to obtain a reliable reference to the original module object, the Reify compiler would inject a variable declaration at the beginning of the generated code that bound some other (uniquely named) variable to the module object, and then that unique variable was used in all the other Reify-generated expressions, such as module1.export(...) and module1.link(...).

The trouble with this strategy was that the injected variable declaration could still collide with other declarations in the module scope that involve the module identifer.

A better way to solve that problem is to introduce another scope by wrapping the module code in an immediately invoked function expression. The original module object can be passed as an argument to that function, and the corresponding parameter can be uniquely named:

(function (module1) {
  module1.export({ name: () => name });
  module1.link("some-module", ...);
  // ...
}).call(this, module);

However, this wrapping should happen as late as possible, in some cases after multiple invocations of the Reify compiler, so the wrapping should not be the responsibility of the Reify compiler itself. Instead, the compiler should just use whatever options.moduleAlias is passed in, and not second-guess it or attempt to inject any variable bindings.

In this PR, the Babel plugin (reify/plugins/babel) becomes responsible for the IIFE wrapping shown above, while also gaining the ability to transform the AST twice: upon entering and upon exiting the Program AST node. This double transformation is necessary to transform both import and export declarations found in the source code, and those that were injected later by other transforms, such as @babel/plugin-transform-runtime.

Before this change, in order to obtain a reliable reference to the
original module object at runtime, the Reify compiler would inject a
variable declaration at the beginning of the generated code that bound
some other (uniquely named) variable to the module object, and then that
unique variable was used in all the other Reify-generated expressions,
such as module1.export(...) and module1.link(...).

The trouble with this strategy was that the injected variable declaration
could still collide with other declarations in the module scope that
involve the `module` identifer.

A better way to solve that problem is to introduce another scope by
wrapping the module code in an immediately invoked function expression.
The original module object can be passed as an argument to that function,
and the corresponding parameter can be uniquely named.

However, this wrapping should happen as late as possible, in some cases
after multiple invocations of the Reify compiler, so the wrapping should
not be the responsibility of the Reify compiler itself. Instead, the
compiler should just use whatever options.moduleAlias is passed in, and
not second-guess it or attempt to inject any variable bindings.
The Reify transform now runs twice: upon entering the Program AST node, to
transform any import/export declarations found in the source code; and
upon exiting the Program node, to transform any import/export declarations
injected by other Babel transforms, e.g. @babel/plugin-transform-runtime.
After adding @babel/plugin-transform-runtime to the plugin pipeline, it's
important to make sure any import declarations it injects into the program
get handled by Reify.
@benjamn benjamn self-assigned this Jun 18, 2019
@@ -1,4 +1,4 @@
"use strict";var module1=module;module1.export({foo:()=>foo});module1.export({id:()=>id,name:()=>name},true);const path = require("path");
"use strict";module.export({foo:()=>foo});module.export({id:()=>id,name:()=>name},true);const path = require("path");
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see from these updated tests, many of the const module1 = module renamings were unnecessary false positives before this change.

@benjamn benjamn merged commit a0e7913 into master Jun 18, 2019
@benjamn benjamn deleted the rename-module-by-wrapping-program-with-IIFE branch June 18, 2019 19:14
benjamn added a commit that referenced this pull request Jun 18, 2019
Reasons for the minor version bump:

  - The Reify compiler is no longer responsible for renaming the module
    identifier to match options.moduleAlias (it just assumes the new
    identifier is defined).

  - The Babel plugin has become responsible for renaming the module
    identifier using an immediately invoked function expression.
    #229

  - The Babel plugin is no longer shipped as a separate npm package, but
    instead lives within the reify package: reify/plugins/babel
    #228
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.

1 participant