-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Support static inline colouring for dynamic import #2295
Conversation
e957024
to
e7360a2
Compare
b508348
to
e7360a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works very nicely!
One minor note: It would be nice to have the dynamic import actually used in the test so that we can see the automatic inlining. You might for instance turn it around—export the dynamic import and make the synchronous import an "empty import". This would also showcase a simple way how to inline specific dynamic imports.
e7360a2
to
e0bc164
Compare
Sure I've switched around the test here, and in the process uncovered a dynamic import inlining bug which I've fixed as well :) |
e0bc164
to
74a0b80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This Pull Request updates dependency [rollup](https://github.com/rollup/rollup) from `v0.61.2` to `v0.62.0` <details> <summary>Release Notes</summary> ### [`v0.62.0`](https://github.com/rollup/rollup/blob/master/CHANGELOG.md#​0620) [Compare Source](rollup/rollup@v0.61.2...v0.62.0) *2018-06-27* * Add option to automatically shim missing exports ([#​2118](`rollup/rollup#2118)) * Inline dynamic imports that are also imported statically and only used in a single chunk ([#​2295](`rollup/rollup#2295)) * Handle caching and invalidation of assets ([#​2267](`rollup/rollup#2267)) * Fix plugin related types ([#​2299](`rollup/rollup#2299)) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
This Pull Request updates dependency [rollup](https://github.com/rollup/rollup) from `v0.61.2` to `v0.62.0` <details> <summary>Release Notes</summary> ### [`v0.62.0`](https://github.com/rollup/rollup/blob/master/CHANGELOG.md#​0620) [Compare Source](rollup/rollup@v0.61.2...v0.62.0) *2018-06-27* * Add option to automatically shim missing exports ([#​2118](`rollup/rollup#2118)) * Inline dynamic imports that are also imported statically and only used in a single chunk ([#​2295](`rollup/rollup#2295)) * Handle caching and invalidation of assets ([#​2267](`rollup/rollup#2267)) * Fix plugin related types ([#​2299](`rollup/rollup#2299)) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
This solves the issue described by @eight04 in #2284 (comment).
If a the time of checking a dynamic import, its already in the same static graph as the parent, then we don't treat it as a dynamic import at that point.
Only if later on we discover that the dynamic import is imported uniquely from a colouring where it is not already in that colouring's static graph do we properly raise it to the status of entry point.