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

[lennartjansson]

When building a bundle with rollup (I have been using experimentalCodeSplitting), and when using an ES6 default import on a module marked as external, rollup will emit a line in the following form at the top of the chunk:

foo = foo && foo.hasOwnProperty('default') ? foo['default'] : foo;

to shim around the fact that the external may be an ES6 module that exports a default member under the default property, or may be a non-ES6 module that expects the entire exported value to be the "default" export.

TypeScript, as of version 2.7, has a flag --esModuleInterop that does a very similar thing. When importing using the import default syntax, TS emits:

foo = __importDefault(foo);

where __importDefault is defined in tslib as

__importDefault = function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};

and modifies the module body to henceforth refer to foo.default instead of foo.

In addition, TS also shims around import * with the tslib helper function

__importStar = function (mod) {
    if (mod && mod.__esModule) return mod;
    var result = {};
    if (mod != null) for (var k in mod) if (Object.hasOwnProperty.call(mod, k)) result[k] = mod[k];
    result["default"] = mod;
    return result;
};

Theoretically, these tslib helpers are supposed to bring TS's compiled output semantics closer to that of babel, but I have no experience with babel personally.

Expected Behavior / Situation

It is possible to get rollup to emit module import shims that exactly match the behavior of TS's shims.

Actual Behavior / Situation

Rollup's default shim is subtly different from TS's __importDefault function. For example, if an external dependency has an exported property called default, but is not built with a tool that adds the __esModule property, rollup will treat the default property as the member to be imported, but TS will not.

Rollup's lack of an import * shim is also subtly different from TS's __importStar function. For example, TS makes it possible to import * as foo from '...' and then refer to foo.default, while rollup doesn't.

Modification Proposal

There are few options here, so I'm curious to hear from the rollup team what you believe to be the best solution here. Some possible options:

• No changes to rollup upstream -- the recommended workaround is to write a transform hook plugin that shims import * lines with a shim compatible with TS's __importStar, and to write a renderChunk hook plugin that shims import default lines with a shim compatible with TS's __importDefault. (The former must be a transform hook since it is difficult to determine which lines in a compiled bundle were originally import *s. The latter must be a renderChunk hook since rollup emits its own default import shim in compiled bundles that must be removed by the plugin.) I've already succeeded at writing a functional implementation of the latter, but it seems like a messy solution overall, especially if we expect others to also encounter this situation.

• We add a new compile option/flag to rollup to optionally make it emit tslib-compatible shims for import default and import * instead of the current behavior. This doesn't break backwards compatibility for existing users, and is a clean solution for anyone else who might need compatibility with TS's esModuleInterop in the future. However, this does involve a code change to rollup hard-coding in a use-case-specific implementation detail.

• We add a config option that allows one to specify the ES import shim implementation that should be emitted in compiled bundles. This means the default implementation could be overriden by the user, and it would be the user's responsibility to pass something compatible with tslib.

• Alternately, if adding a new config option with a template or code-generation pattern is deemed too heavyweight of an interface, there could be a new boolean config option that merely disables the existing default import shim, where one would need to continue to rely on rollup plugins to codemod TS-compatible shims back into the compiled bundles.

Look forward to hearing your thoughts!

[taylorcode]

I'm also running into this problem, for example:

import * as Thing from 'thing';

const RealThing = Thing.default;

Will break when it is rollupd.

[dgoldstein0]

That's mentioned in the issue already:

Rollup's lack of an import * shim is also subtly different from TS's

__importStar function. For example, TS makes it possible to import * as foo from '...' and then refer to foo.default, while rollup doesn't.

…

On Mon, Nov 19, 2018, 5:37 PM Taylor McIntyre ***@***.***> wrote: I'm also running into this problem, for example: import * as Thing from 'thing'; const RealThing = Thing.default; Will break when it is rollupd. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2554 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBnd-9yh3J96ejj3UpG8o9BeWYC6Hfsks5uwzK3gaJpZM4Yaoke> .

[joonhocho]

Just ran into this issue.

[shellscape]

Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity.

We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @shellscape and request a re-open, and we'll be happy to oblige.

[lukastaegert]

I actually think a lot about these things an hope we can align them at some point. The diverging behaviour around default imports was created to handle cases where people put namespaces on global variables (but I would need to check if I find the old issue, doing this from my phone). Keeping this open.

[dgoldstein0]

realized I meant to reply to the last message but let it get buried in my email instead. Sorry for being slow on this.

For a little context, I work with Lennart, who originally filed the issue.

Our ask here isn't necessarily to change rollup's default behavior - while we'd certainly be happy with that, we would also be very happy with a solution that let us have reasonable hooks for a plugin we could develop to make rollup & typescript behave the same. Our use case is to use rollup for production as an optimization pass, but leave it out of development to keep our edit-compile-refresh times faster, so it's important we make those cases have the same behavior. (that said, our team shelved the project for a while, hence the silence from our part. But we plan to pick it back up next year).

Re-reading the original post, I think the original set of options are approximately right, though a few more things to add:

• we only need to change the import * and import default behavior for externalized modules (or transformed modules in some cases... but we're not planning on using other transformer plugins); as the typescript --esModuleInterop flag only conceptually changes the behavior of non ES6 modules. The first proposed solution - using a transform hook for inserting __importStar - seems likely to emit extra boilerplate that rollup & uglify won't be quite smart enough to remove, so is non-ideal.
• rollup has a lot of callback-based hooks for plugins; a reasonable version of the third solution could be some callback for rollup to use for shimming import * and import default of externalized modules
• a caveat missed in the original post is that dynamic import()s are also affected - import("foo") under typescript's --esModuleInterop is compiled to something equivalent to import("foo").then(__importStar) (when --module is AMD or commonjs). I suppose the first option could do this code modding in the transform hook, but that has the same issue with potentially inserting unnecessary __importStar calls.

[dgoldstein0]

Pinging here - can we get some discussion going again? I work with Lennart, and while the original project we were using rollup for got put on hold, we're going to be using rollup for a different project now, and foresee this exact same problem.

One reason our requirements are unique is that we're using typescript as a separate build step - typescript and rollup are both just tools managed by our larger build system; this gives us enormous speed benefits as our build system can cache the compiled typescript (both locally & remotely). I saw there are a few different rollup plugins that mention typescript support on https://github.com/rollup/awesome, but they all want to do the typescript compilation themselves, so right away don't meet our needs. I suspect that those plugins might not handle esModuleInterop, as I didn't see it mentioned in their code, but I could be wrong.

So our options for making rollup generate AMD from typescript compiled with --esModuleInterop seem to be:

1. rollup solves this issue natively (perhaps behind a config flag). we would love this, but it's not required.
2. rollup provides hooks for an extension to solve this.
3. work around rollup's lack of a solution by generate ES modules with rollup, and run the rollup outputs through typescript. This is very doable, but also a slow option as it involves re-parsing the output.

so you might ask, could we create an extension today? Yes... but it's hacky enough to not be worth it:

• We were already able to create a hacky extension to work around the import default issue by identifying rollups default import shim code and replacing it. This is over-complicated, as it involves parsing the output code in it's entirety. Given this, compiling to Ecmascript modules and running through typescript.transpileModule may be the better option (there won't be a significant speed different, and typescript's code gen is better tested).
• we haven't yet solved the import * from 'externalized_module' case. Doing it as an output hook seems to be a nonstarter since at that level, we can't tell if the code was generated from an import * or some other style of import (e.g import {foo}. A build hook for transforming import * usage could work if we filter to only messing with externalized modules' imports. Dynamic imports should also be a similar story.
• That said, while we could try a build hook for import default, we have no way to disable the existing codegen that creates foo = foo && foo.hasOwnProperty('default') ? foo['default'] : foo;

so... I suppose my conclusion is that until there are better hooks available, our sanest option is to use typescript's transpile or transpileModule api on the rollup output, and generate ES6 from rollup.

The main hooks I would like to see would be for a plugin (or the rollup config... doesn't really matter to me) be able to specify functions to use to wrap imported externalized modules; for typescript, we'd want to use tslib.__importDefault for default imports, and tslib.__importStar for import * and dynamic imports. Ideally typescript would also make it easy to inject the tslib dependency if it's not already there.

[dgoldstein0]

so... I suppose my conclusion is that until there are better hooks available, our sanest option is to use typescript's transpile or transpileModule api on the rollup output, and generate ES6 from rollup.

correction... since typescript doesn't support input sourcemaps, this isn't really an option. So I guess we'll be hacking together some sort of super messy plugin.

[lukastaegert]

Sorry for the long silence. So to go back to the original issue:

• I think we could just add a start import shim like TypeScript as I do not think it would be a breaking change
• We could add an option to use the TS default import shim. The problem here is that while this shim is superior in some way as it supports live bindings, it means we need to write accesses to the default import now as property accesses. This will be a bigger refactoring. The default value of such an option would be so that we have the old behaviour for now of course to avoid the breaking change, but we could switch it with the next major to align with the rest of the ecosystem.

[dgoldstein0]

I think we could just add a start import shim like TypeScript as I do not

think it would be a breaking change care to clarify what you mean by this? also you are only addressing half the problem - the import default problem - whereas dynamic imports and import * also have differing semantics under typescript's esModuleInterop. That might be more clearly in scope of this discussion if it were included in the title.

…

On Fri, Mar 13, 2020 at 12:44 AM Lukas Taegert-Atkinson < ***@***.***> wrote: Sorry for the long silence. So to go back to the original issue: - I think we could just add a start import shim like TypeScript as I do not think it would be a breaking change - We could add an option to use the TS default import shim. The problem here is that while this shim is superior in some way as it supports live bindings, it means we need to write accesses to the default import now as property accesses. This will be a bigger refactoring. The default value of such an option would be so that we have the old behaviour for now of course to avoid the breaking change, but we could switch it with the next major to align with the rest of the ecosystem. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2554 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIGO5ZHTLDNF3R74XG6UADRHHBZ5ANCNFSM4GDKREPA> .

[dgoldstein0]

Cycling back here. I'm still hopeful to see some progress on this.

re my last comment, I don't think we need to bikeshed about whether this is a breaking change, as long as we mention it in the release notes.

I also don't think it's very important to make it possible to use typescript's shims vs. our own, as long as they have equivalent behavior.

We could add an option to use the TS default import shim. The problem here is that while this shim is superior in some way as it supports live bindings, it means we need to write accesses to the default import now as property accesses. This will be a bigger refactoring. The default value of such an option would be so that we have the old behaviour for now of course to avoid the breaking change, but we could switch it with the next major to align with the rest of the ecosystem.

I don't think I totally understood this before, but I do now. I don't think live-changing default exports is very common, so the MVP doesn't need to support this. This is a slight behavior divergence, but seems acceptable for making some progress now. The more important part is that today, if a module has a .default but not .__esModule = true or vice versa, different tools will interpret it in very different ways.

So it sounds like step 1 is to mirror the typescript behavior, with the caveat of not supporting live-updating bindings. Does that sound accurate? If so I'm happy to take a stab at it and send you folks a PR.

[dasa]

tslib 2.0 and typescript 3.9 now support live bindings as well: microsoft/tslib#78

[homburg]

I ran into this problem with the winston package, when setting up a new project with babel-typescript.

This plugin: https://github.com/goto-bus-stop/rollup-plugin-es-module-interop/ solves the issue perfectly for me.

But native support matching babel and typescript would be nice, to avoid suprising behavior and extra setup.

[lukastaegert]

Sorry for the long silence. I finally decided to give these things highest priority to fix (mostly) all interop issues in reasonable ways. I created a discussion and overview issue here: rollup/plugins#481

[lukastaegert]

I believe #3710 will provide a fix for this on Rollup side

[dgoldstein0]

wow, just read #3710 and it looks like --interop auto is exactly what we wanted.

I think there's one other rollup+typescript interop issue I encountered in practice, but it's not something described here (it has to do with export behavior, not import behavior). I may file it as a separate issue.