Skip to content

Support tree-shakable imports for --target es2015#32742

Merged
rbuckton merged 2 commits into
masterfrom
fix16999
Aug 9, 2019
Merged

Support tree-shakable imports for --target es2015#32742
rbuckton merged 2 commits into
masterfrom
fix16999

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton commented Aug 6, 2019

Changes the emit for --importHelpers when the module kind is es2015 or esnext so that they are tree-shakable by WebPack.

For example, we previously would emit:

// --module es2015
import * as tslib_1 from "tslib";
const obj = tslib_1.__assign({}, other);

But now we emit:

// --module es2015
import { __assign } from "tslib";
const obj = __assign({}, other);

The emit for commonjs, amd, umd, and system is unaffected.

Fixes #16999

Copy link
Copy Markdown
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Can I see a test where we have --importHelpers and something like:

const obj = {};
const __spread = {...obj};

(so there's a local name which takes priority over the helper name)
I'd expect our emit to be something like

import { __spread as __spread_1 } from "tslib";
const x = {};
const __spread = __spread_1({}, x);

(where previously no renaming of the helper would be needed, since it was namespaced)

@rbuckton
Copy link
Copy Markdown
Contributor Author

rbuckton commented Aug 7, 2019

I'd expect our emit to be something like [...]

We explicitly don't do this for any helpers, because we allow you to replace helpers. If we renamed a helper because we found a declaration with the same name at the top-level, we would break that feature.

@weswigham
Copy link
Copy Markdown
Member

We explicitly don't do this for any helpers, because we allow you to replace helpers. If we renamed a helper because we found a declaration with the same name at the top-level, we would break that feature.

But that doesn't work for import helpers, because the helper gets imported and accessed via namespace (usually)?

@rbuckton
Copy link
Copy Markdown
Contributor Author

rbuckton commented Aug 8, 2019

"Rename the helper for all uses of the helper in the file but only if its imported and --target ES2015, but don't rename it if its emitted inline" seems kind of an esoteric rule that I'm not sure is 100% necessary.

@weswigham
Copy link
Copy Markdown
Member

weswigham commented Aug 8, 2019

I'm just pointing out that previously

const x = {};
const __assign = {...x, x: 12};
export {};

with importHelpers: true, target: es5, and module: es6 would work at runtime without error, while after the import style change, its behavior changes and now __assign mangles the import...

@rbuckton
Copy link
Copy Markdown
Contributor Author

rbuckton commented Aug 8, 2019

Fair enough. I'll look into it tomorrow.

@rbuckton
Copy link
Copy Markdown
Contributor Author

rbuckton commented Aug 9, 2019

I've added import aliases for cases where there are collisions.

@rbuckton rbuckton merged commit 05af8fa into master Aug 9, 2019
@rbuckton rbuckton deleted the fix16999 branch August 9, 2019 21:02
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make tslib imports shakable

2 participants