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

Support multiple entry inputs #410

Merged
merged 11 commits into from
May 28, 2019
Merged

Support multiple entry inputs #410

merged 11 commits into from
May 28, 2019

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented May 28, 2019

This implements #113 supporting an object form for entry:

ncc({
  entry1: '/path/to/entry1.js',
  entry2: '/path/to/entry2.js'
})

where the separate entry points will have shared chunks and therefore shared dependency instances meaning any shared dependency state will be maintained as shared state between the entry points (no more instancing issues!).

In addition filename supports the Webpack [name] patterns now.

I also changed the output API to support this to an object with an output property (this is what we did in Rollup!), where the output is an object mapping file path strings to file entries or symlinks:

ncc().then(({ files }) => {
  console.log(files['index.js'].source);
});

I'm not fixed to this API thought at all, so please do share your feedback on this if you think any variations may be better.

This PR is passing all tests tests when combined with the PR at vercel/webpack-asset-relocator-loader#43, and vercel/webpack-asset-relocator-loader#44.

Closes #113

@guybedford guybedford requested a review from styfle as a code owner May 28, 2019 13:01
@styfle
Copy link
Member

styfle commented May 28, 2019

@guybedford The WARL pr looks good, lets bump the dependency here.

@guybedford
Copy link
Contributor Author

Ok the fix turned out to be vercel/webpack-asset-relocator-loader#44, as soon as that is merged, I can bump the asset loader here.

@guybedford
Copy link
Contributor Author

I've also added the feature to this PR to support multiple entry points in the CLI via:

ncc build input1.js input2.js -o dist

readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
@guybedford guybedford force-pushed the multi-file branch 2 times, most recently from d073c16 to e1aefeb Compare May 28, 2019 14:43
readme.md Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented May 28, 2019

Codecov Report

Merging #410 into master will decrease coverage by 1.02%.
The diff coverage is 83.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #410      +/-   ##
==========================================
- Coverage   74.68%   73.65%   -1.03%     
==========================================
  Files          13       13              
  Lines         395      391       -4     
==========================================
- Hits          295      288       -7     
- Misses        100      103       +3
Impacted Files Coverage Δ
src/index.js 83.81% <76.66%> (+0.27%) ⬆️
src/cli.js 59.87% <96.55%> (-2.49%) ⬇️
src/utils/get-package-base.js 0% <0%> (-37.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf98881...332602b. Read the comment docs.

readme.md Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
});
}
else if (typeof entry === 'string') {
processedEntry['index.js'] = pathResolve(entry);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
processedEntry['index.js'] = pathResolve(entry);
processedEntry[entry] = pathResolve(entry);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can change the default to the basename, but the default filename for any string value input right now is index.js.

readme.md Show resolved Hide resolved
readme.md Show resolved Hide resolved
Co-Authored-By: Steven <[email protected]>
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Ok looks good. Let's release another beta.

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.

Supporting multiple entry points
3 participants