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

Import specifier deshadowing fix #1912

Merged
merged 1 commit into from
Jan 26, 2018

Conversation

guybedford
Copy link
Contributor

This fixes #1910.

The main fix is just the line in src/Chunk.ts, but in the process this exposed an idempotency bug in the deshadowing process itself in that safeName results from previous deshadowing runs can affect future runs.

I've fixed the idempotency bug with a reliable but hacky approach for now, hopefully we can put some thought to something better here, but correctness comes first I feel. Just let me know if I can help explain it at all further.

@guybedford
Copy link
Contributor Author

(To explain the idempotency bug - it is that the iife format run which comes after the es run would have Sticky$$1 as the function deshadowing name, when the deshadowing is unnecessary for this render run, since the safeName would have been left over from a previous run. Ideally the safeName data would be a separate layer or entirely cleared between runs, but a full reset iteration seems like a performance loss.)

this.safeName[this.name.length + 1] === '$') {
this.safeName = undefined;
return this.name;
}
Copy link
Member

Choose a reason for hiding this comment

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

I see what you mean. I think this is not too bad for now but yes, this is a weird interaction between different runs we should probably fix more substantially at some point. Good catch, though 👍

@lukastaegert lukastaegert added this to the 0.55.1 milestone Jan 25, 2018
@lukastaegert lukastaegert changed the base branch from master to release-0.55.1 January 26, 2018 05:51
@lukastaegert lukastaegert merged commit bb3c49f into release-0.55.1 Jan 26, 2018
@lukastaegert lukastaegert deleted the import-specifier-deshadowing branch January 27, 2018 09:05
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.

None yet

2 participants