Rework interop handling #3710

lukastaegert · 2020-08-10T05:51:48Z

This PR contains:

Are tests included?

yes (bugfixes and features will not be merged without tests)
no

Breaking Changes?

yes (breaking changes will not be merged unless absolutely necessary)
no

List any relevant issue numbers:
Resolves #3288
Resolves #2554

Description

This completely reimplements how interop is handled. I.e. in formats cjs, amd, iife and umd, this reimplements how default and namespace imports and reexports are handled, as well as what is returned by dynamic imports from external dependencies.

Besides the existing values, there are now four new interop values: "auto", "esModule", "default" and "defaultOnly". Moreover, a function can be supplied for output.interop to configure the correct interop per dependency.

From the new documentation:

output.interop

Controls how Rollup handles default, namespace and dynamic imports from external dependencies in formats like CommonJS that do not natively support these concepts. Note that even though true is the current default value, this value is deprecated and will be replaced by "auto" in the next major version of Rollup. In the examples, we will be using the CommonJS format, but the interop similarly applies to AMD, IIFE and UMD targets as well.

To understand the different values, assume we are bundling the following code for a cjs target:

import ext_default, * as external from 'external1';
console.log(ext_default, external.bar, external);
import('external2').then(console.log);

Keep in mind that for Rollup, import * as ext_namespace from 'external'; console.log(ext_namespace.bar); is completely equivalent to import {bar} from 'external'; console.log(bar); and will produce the same code. In the example above however, the namespace object itself is passed to a global function as well, which means we need it as a properly formed object.

"esModule" assumes that required modules are transpiled ES modules where the required value corresponds to the module namespace and the default export is the .default property of the exported object:
```
var external = require('external1');
console.log(external['default'], external.bar, external);
Promise.resolve().then(function () {
  return require('external2');
}).then(console.log);
```
When esModule is used, Rollup adds no additional interop helpers and also supports live-bindings for default exports.

"default" assumes that the required value should be treated as the default export of the imported module, just like when importing CommonJS from an ES module context in Node. In contrast to Node, though, named imports are supported as well which are treated as properties of the default import. To create the namespace object, Rollup injects helpers:

var external = require('external1');

function _interopNamespaceDefault(e) {
  var n = Object.create(null);
  if (e) {
    Object.keys(e).forEach(function (k) {
      if (k !== 'default') {
        var d = Object.getOwnPropertyDescriptor(e, k);
        Object.defineProperty(n, k, d.get ? d : {
          enumerable: true,
          get: function () {
            return e[k];
          }
        });
      }
    });
  }
  n['default'] = e;
  return Object.freeze(n);
}

var external__namespace = /*#__PURE__*/_interopNamespaceDefault(external);
console.log(external, external.bar, external__namespace);
Promise.resolve().then(function () {
  return /*#__PURE__*/_interopNamespaceDefault(require('external2'));
}).then(console.log);

"auto" combines both "esModule" and "default" by injecting helpers that contain code that detects at runtime if the required value contains the __esModule property. Adding this property is a standard implemented by Rollup, Babel and many other tools to signify that the required value is the namespace of a transpiled ES module:

var external = require('external1');

function _interopNamespace(e) {
  if (e && e.__esModule) { return e; } else {
    var n = Object.create(null);
    if (e) {
      Object.keys(e).forEach(function (k) {
        if (k !== 'default') {
          var d = Object.getOwnPropertyDescriptor(e, k);
          Object.defineProperty(n, k, d.get ? d : {
            enumerable: true,
            get: function () {
              return e[k];
            }
          });
        }
      });
    }
    n['default'] = e;
    return Object.freeze(n);
  }
}

var external__namespace = /*#__PURE__*/_interopNamespace(external);
console.log(external__namespace['default'], external.bar, external__namespace);
Promise.resolve().then(function () {
  return /*#__PURE__*/_interopNamespace(require('external2'));
}).then(console.log);

Note how Rollup is reusing the created namespace object to get the default export. If the namespace object is not needed, Rollup will use a simpler helper:

// input
import ext_default from 'external';
console.log(ext_default);

// output
var ext_default = require('external');

function _interopDefault (e) { return e && e.__esModule ? e : { 'default': e }; }

var ext_default__default = /*#__PURE__*/_interopDefault(ext_default);
console.log(ext_default__default['default']);

"defaultOnly" is similar to "default" except for the following:
- Named imports are forbidden. If such an import is encountered, Rollup throws an error even in es and system formats. That way it is ensure that the es version of the code is able to import non-builtin CommonJS modules in Node correctly.
- While namespace reexports export * from 'external'; are not prohibited, they are ignored and will cause Rollup to display a warning because they would not have an effect if there are no named exports.
- When a namespace object is generated, Rollup uses a much simpler helper.
Here is what Rollup will create from the example code. Note that we removed external.bar from the code as otherwise, Rollup would have thrown an error because, as stated before, this is equivalent to a named import.
```
var ext_default = require('external1');

function _interopNamespaceDefaultOnly(e) {
  return Object.freeze({__proto__: null, 'default': e});
}

var ext_default__namespace = /*#__PURE__*/_interopNamespaceDefaultOnly(ext_default);
console.log(ext_default, ext_default__namespace);
Promise.resolve().then(function () {
  return /*#__PURE__*/_interopNamespaceDefaultOnly(require('external2'));
}).then(console.log);
```

When a function is supplied, Rollup will pass each external id to this function once to control the interop type per dependency.

As an example if all dependencies are CommonJs, the following config will ensure that named imports are only permitted from Node builtins:

// rollup.config.js
import builtins from 'builtins';
const nodeBuiltins = new Set(builtins());

export default {
  // ...
  output: {
    // ...
    interop(id) {
      if (nodeBuiltins.has(id)) {
        return 'default';
      }
      return 'defaultOnly';
    }
  }
};

true is equivalent to "auto" except that it uses a slightly different helper for the default export that checks for the presence of a default property instead of the __esModule property.

☢️ This value is deprecated and will be removed in a future Rollup version.
false is equivalent to using default when importing a default export and esModule when importing a namespace.

☢️ This value is deprecated and will be removed in a future Rollup version.

There are some additional options that have an effect on the generated interop code:

Setting output.exernalLiveBindings to false will generate simplified namespace helpers as well as simplified code for extracted default imports.
Setting output.freeze to false will prevent generated interop namespace objects from being frozen.

Included fixes and improvements

Features

Allow defining interop per dependency via a function (Rework interop handling #3710)
Support interop "auto" as a more compatible version of "true" (Rework interop handling #3710)
Support interop "default" and "esModule" to avoid unnecessary interop helpers (Rework interop handling #3710)
Support interop "defaultOnly" for simplified helpers and Node ESM interop compatible output (Rework interop handling #3710)
Respect interop option for external dynamic imports (Rework interop handling #3710)
Support live-bindings for external default imports in non-ES formats unless "externalLiveBindings" is "false" (Rework interop handling #3710)
Use shared default interop helpers for AMD, UMD and IIFE formats (Rework interop handling #3710)
Avoid unnecessarily deconflicted module variables in non-ES formats (Rework interop handling #3710)
Freeze generated interop namespace objects (Rework interop handling #3710)
Always mark interop helpers as pure (Rework interop handling #3710)
Avoid default export interop if there is already an interop namespace object (Rework interop handling #3710)
Sort all require statements to the top in CommonJS output for easier back-transpilation to ES modules by other tools (Rework interop handling #3710)

Bug Fixes

Deconflict the names of helper variables introduced for interop (Rework interop handling #3710)
Generate proper namespace objects for static namespace imports in non-ES formats (Rework interop handling #3710)
Do not add unused interop helpers when using the renderDynamicImport hook (Rework interop handling #3710)

rollup-bot · 2020-08-10T11:16:58Z

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#better-default-interop

or load it into the REPL:
https://rollupjs.org/repl/?circleci=12506

codecov · 2020-08-10T11:18:25Z

Codecov Report

Merging #3710 into master will increase coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3710      +/-   ##
==========================================
+ Coverage   96.80%   96.96%   +0.16%     
==========================================
  Files         183      184       +1     
  Lines        6320     6402      +82     
  Branches     1843     1854      +11     
==========================================
+ Hits         6118     6208      +90     
+ Misses        105      103       -2     
+ Partials       97       91       -6

Impacted Files	Coverage Δ
src/utils/variableNames.ts	`100.00% <ø> (ø)`
cli/run/batchWarnings.ts	`98.40% <100.00%> (-0.03%)`	⬇️
src/Chunk.ts	`100.00% <100.00%> (ø)`
src/ExternalModule.ts	`100.00% <100.00%> (+2.04%)`	⬆️
src/ast/nodes/ImportExpression.ts	`100.00% <100.00%> (ø)`
src/ast/nodes/MetaProperty.ts	`100.00% <100.00%> (ø)`
src/ast/nodes/ObjectExpression.ts	`92.80% <100.00%> (+0.91%)`	⬆️
src/ast/scopes/ChildScope.ts	`100.00% <100.00%> (ø)`
src/ast/scopes/ModuleScope.ts	`100.00% <100.00%> (+11.76%)`	⬆️
src/ast/utils/PathTracker.ts	`100.00% <100.00%> (ø)`
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cd3094...188ad34. Read the comment docs.

guybedford

Do ping when you've got the docs up.

guybedford · 2020-08-10T20:24:21Z

test/form/samples/deprecated/interop-false-reexport/_expected/amd.js

@@ -2,7 +2,7 @@ define(['exports', 'external'], function (exports, external) { 'use strict';



-	exports.p = external.default;
+	exports.p = external;


Is this a bug fix!?

As a matter of fact yes, here, reexports behaved differently from regular imports where the .default was not added.

lukastaegert · 2020-08-11T04:59:58Z

@guybedford I added the documentation and also pasted it into the PR description. By tomorrow, I will try to compile what other changes are included as some hidden bugs were fixed as well.

lukastaegert · 2020-08-12T04:14:02Z

I also compiled the changelog now, this is a list of the included fixes and improvements:

Features

Allow defining interop per dependency via a function (Rework interop handling #3710)
Support interop "auto" as a more compatible version of "true" (Rework interop handling #3710)
Support interop "default" and "esModule" to avoid unnecessary interop helpers (Rework interop handling #3710)
Support interop "defaultOnly" for simplified helpers and Node ESM interop compatible output (Rework interop handling #3710)
Support live-bindings for external default imports in non-ES formats unless "externalLiveBindings" is "false" (Rework interop handling #3710)
Use shared default interop helpers for AMD, UMD and IIFE formats (Rework interop handling #3710)
Avoid unnecessarily deconflicted module variables in non-ES formats (Rework interop handling #3710)
Freeze generated interop namespace objects (Rework interop handling #3710)
Always mark interop helpers as pure (Rework interop handling #3710)
Avoid default export interop if there is already an interop namespace object (Rework interop handling #3710)

Bug Fixes

Deconflict the names of helper variables introduced for interop (Rework interop handling #3710)
Generate proper namespace objects for static namespace imports in non-ES formats (Rework interop handling #3710)
Do not add unused interop helpers when using the renderDynamicImport hook (Rework interop handling #3710)

lukastaegert · 2020-08-12T05:25:56Z

This is now complete and ready for review
cc @guybedford

dgoldstein0 · 2020-08-12T08:04:27Z

very impressive list of changes! Definitely looking forward to this.

The description mentions that dynamic imports are impacted too, but doesn't give any examples of the codegen changes for those. could you add more details around that? I expect it'll be the same as the import * changes, but would like some confirmation.

lukastaegert · 2020-08-12T09:12:41Z

Yes, exactly. Maybe I extend the examples to also contain a dynamic import as it would of course use the same helper.

guybedford

This is some really great work and a solid model!

One extension of auto in future that I always imagined would be interesting for __esModule would be to check ns[Symbol.toStringTag] === 'Module' as equivalent to the __esModule branding check. This way it could be possible to have a autoModern or something mode which does "the right thing" for externals in both Node.js and browsers without needing the per-module function to support that.

guybedford · 2020-08-12T21:14:02Z

Also, the modern module brand check can be done with Object.prototype.toString.call(ns) === '[object Module]' to support legacy environments properly.

lukastaegert · 2020-08-13T04:37:22Z

@guybedford That is definitely an interesting idea. If I understand correctly, though, it would involve Rollup's cjs/amd/iife/umd output to replace or augment

Object.defineProperty(exports, '__esModule', { value: true });

with

exports[Symbol.toStringTag] = 'Module';

or alternatively

Object.defineProperty(exports, 'toString', { value: function() { return  '[object Module]'; } });

for a legacy version to work correctly?

@dgoldstein0 I extended the examples to also contain a dynamic import as well, I hope this answers your question.

guybedford · 2020-08-13T06:07:57Z

@lukastaegert actually thinking about this more I'm not sure it solves anything... sorry for false suggestion. I don't think we should ever change the __esModule brand because that is a reliable differentiator. Rather this was about environment inference, but in an ES module environment we just want to be treating everything as a namespace anyway so perhaps best not to meddle.

dgoldstein0 · 2020-08-13T06:32:39Z

@dgoldstein0 I extended the examples to also contain a dynamic import as well, I hope this answers your question.

yes it does, thanks a ton!

…handling

… more cases

…freeze interop namespaces, mark interop helpers as pure

…rn when reexporting the namespace

includes workaround for issues introduced in rollup/rollup#3710

guybedford reviewed Aug 10, 2020

View reviewed changes

This was referenced Aug 12, 2020

interopNamespace added when interop is disabled #3288

Closed

Rollup's external default import shim is not entirely compatible with TypeScript's --esModuleInterop #2554

Closed

lukastaegert force-pushed the better-default-interop branch from e651d11 to 2c02522 Compare August 12, 2020 05:24

lukastaegert marked this pull request as ready for review August 12, 2020 05:25

guybedford approved these changes Aug 12, 2020

View reviewed changes

lukastaegert force-pushed the better-default-interop branch from f67663a to b06eb19 Compare August 13, 2020 04:31

lukastaegert added 12 commits August 13, 2020 21:33

Use interop default helper for AMD, IIFE, UMD as well, unify interop …

e60a56d

…handling

Support live-bindings for default imports, deconflict default variables

39ecb40

Only deconflict namespace variables in ESM when preserving modules

332a80a

Do not deconflict default export variables if there is no interop

d96e2a8

Remove unneeded checks, use square brackets for default properties in…

ca077d6

… more cases

Fix external dependency variable name deconflicting

3b828f5

Generate proper interop namespaces when losing track of a namespace, …

3016d88

…freeze interop namespaces, mark interop helpers as pure

Decouple dynamic import deconflicting per output

0dea6b5

Implement per dependency interop

7c15a77

Support "auto" interop

b787e21

Unify export block generation

9273ac0

Take default from namespace if possible

5625610

lukastaegert added 12 commits August 13, 2020 21:35

Use static default interop if externalLiveBindings are false

27b4d7b

Use correct namespace interop for default case

61e2f82

Add defaultOnly interop type

d08e5a8

Deprecate boolean interop values

d1c500a

Throw for invalid interop values

3ebcc7c

Throw when importing named bindings from a defaultOnly dependency, wa…

440a66c

…rn when reexporting the namespace

Do not add unused helpers for custom dynamic import wrappers

6b88f2d

Add documentation

6901a57

Update changelog

4f22c80

Improve docs

d41a16f

Improve coverage

7a7e15f

Add dynamic import to examples

188ad34

lukastaegert force-pushed the better-default-interop branch from b06eb19 to 188ad34 Compare August 13, 2020 19:37

lukastaegert merged commit 4b41f4e into master Aug 13, 2020

lukastaegert deleted the better-default-interop branch August 13, 2020 19:43

lukastaegert mentioned this pull request Aug 15, 2020

[commonjs] Discussion and overview of (nearly) all current interop issues + ideas how to move forward rollup/plugins#481

Closed

seleb added a commit to seleb/bitsy-hacks that referenced this pull request Aug 30, 2020

fix: upgrade dependencies

1bfa52a

includes workaround for issues introduced in rollup/rollup#3710

wessberg mentioned this pull request Sep 2, 2020

Missing __importStart and __importDefault calls when synthetic is required wessberg/rollup-plugin-ts#113

Closed

luckyrat mentioned this pull request Oct 22, 2020

Investigate rollup-common-js bug kee-org/browser-addon#285

Closed

wardpeet mentioned this pull request Mar 26, 2021

enhance: support tsconfig esmodule interop developit/microbundle#785

Closed

bluwy mentioned this pull request Jul 10, 2022

2 imports run for same module after vite build vitejs/vite#9018

Closed

7 tasks

markerikson mentioned this pull request Mar 20, 2023

Update Rollup to 3.x facebook/react#26442

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework interop handling #3710

Rework interop handling #3710

lukastaegert commented Aug 10, 2020 •

edited

Loading

rollup-bot commented Aug 10, 2020 •

edited

Loading

codecov bot commented Aug 10, 2020 •

edited

Loading

guybedford left a comment

guybedford Aug 10, 2020

lukastaegert Aug 11, 2020

lukastaegert commented Aug 11, 2020

lukastaegert commented Aug 12, 2020

lukastaegert commented Aug 12, 2020

dgoldstein0 commented Aug 12, 2020

lukastaegert commented Aug 12, 2020

guybedford left a comment

guybedford commented Aug 12, 2020

lukastaegert commented Aug 13, 2020

guybedford commented Aug 13, 2020

dgoldstein0 commented Aug 13, 2020

		@@ -2,7 +2,7 @@ define(['exports', 'external'], function (exports, external) { 'use strict';



		exports.p = external.default;
		exports.p = external;

Rework interop handling #3710

Rework interop handling #3710

Conversation

lukastaegert commented Aug 10, 2020 • edited Loading

Description

output.interop

Included fixes and improvements

Features

Bug Fixes

rollup-bot commented Aug 10, 2020 • edited Loading

Thank you for your contribution! ❤️

codecov bot commented Aug 10, 2020 • edited Loading

Codecov Report

guybedford left a comment

Choose a reason for hiding this comment

guybedford Aug 10, 2020

Choose a reason for hiding this comment

lukastaegert Aug 11, 2020

Choose a reason for hiding this comment

lukastaegert commented Aug 11, 2020

lukastaegert commented Aug 12, 2020

Features

Bug Fixes

lukastaegert commented Aug 12, 2020

dgoldstein0 commented Aug 12, 2020

lukastaegert commented Aug 12, 2020

guybedford left a comment

Choose a reason for hiding this comment

guybedford commented Aug 12, 2020

lukastaegert commented Aug 13, 2020

guybedford commented Aug 13, 2020

dgoldstein0 commented Aug 13, 2020

lukastaegert commented Aug 10, 2020 •

edited

Loading

rollup-bot commented Aug 10, 2020 •

edited

Loading

codecov bot commented Aug 10, 2020 •

edited

Loading