Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Add support for experimentalCodeSplitting from Rollup 0.55.0 #283

Merged
merged 3 commits into from
Jan 26, 2018

Conversation

jthoms1
Copy link
Contributor

@jthoms1 jthoms1 commented Jan 25, 2018

This pull request is to close issue #282. The code update includes changes to allow for an array of file paths as well as a single string for input/entry. I have also added tests to ensure we have coverage.

As part of this PR I had to upgrade the project's rollup dependency to 0.55. It appears that in doing so a test was failing for sourcemap paths. If a sourcemapFile is supplied then the internal sourcemap paths should be relative.

@kristianmandrup
Copy link

Please merge this PR ASAP. It is currently blocking a critical update to stencilJS, in order to make the 1.0 release with full ES module support. Thanks :)

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Great fix, thanks a lot! I'll release it once I had a look at the remaining open PRs.

src/index.js Outdated
entryModuleIds.push( resolved );
})
)
);
Copy link
Member

Choose a reason for hiding this comment

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

Minor totally unimportant nit: This looks a little more confusing as it needs to be (and it can possibly shuffle the order of the entry points which may or may not cause someone to stumble over this again in the future). If you Promise.all the mapped entryModules first and then assign the resulting array to entryModuleIds, the order would be preserved and one would not stop for a second to wonder if entryModuleIds really is a permuted map of entryModules 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally fair, I have updated the code to be more succinct and to use the result of the entryModuleIdsPromise.

src/index.js Outdated
@@ -173,7 +168,7 @@ export default function commonjs ( options = {} ) {
if ( !filter( id ) ) return null;
if ( extensions.indexOf( extname( id ) ) === -1 ) return null;

return entryModuleIdsPromise.then( () => {
return entryModuleIdsPromise.then( (entryModuleIds) => {
Copy link
Member

Choose a reason for hiding this comment

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

That was quick. Yes, that's even better!

@lukastaegert lukastaegert merged commit 370bf07 into rollup:master Jan 26, 2018
@lukastaegert lukastaegert added this to the 8.4.0 milestone Jan 26, 2018
@kristianmandrup
Copy link

Awesome!!!

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.

None yet

3 participants