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

Bundle node modules on --target=node/electron #796

Closed

Conversation

lbguilherme
Copy link
Contributor

This adds the option --bundle-node-modules to force bundling all files even on --target=node. This does not work with all modules , but should work for most simpler cases. It will break apart if a module try to do filesystem operations or depend on some internal files. Things such as puppeteer or lsmod will certainly fail.

This is ready to merge (please review!).

In a followup pull request I'll bring support for native node modules. Then we can start checking what packages don't build and what can be done for them.

This still isn't capable of bundling Parcel itself, but eventually it should be.


The major change is a rewrite of the Resolver, that now have to conditionally handle pkg.browser (the old resolver had no way to disable that). The other files touched are minimal.

@codecov-io
Copy link

Codecov Report

Merging #796 into master will decrease coverage by 4.3%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #796      +/-   ##
==========================================
- Coverage   95.52%   91.21%   -4.31%     
==========================================
  Files          65       66       +1     
  Lines        2188     2926     +738     
==========================================
+ Hits         2090     2669     +579     
- Misses         98      257     +159
Impacted Files Coverage Δ
src/Bundler.js 89.71% <ø> (-4.74%) ⬇️
src/Resolver.js 100% <100%> (ø) ⬆️
src/builtins/index.js 100% <100%> (ø) ⬆️
src/visitors/dependencies.js 93.24% <100%> (-0.1%) ⬇️
src/assets/JSAsset.js 95.38% <100%> (-4.62%) ⬇️
src/HMRServer.js 52.63% <0%> (-34.33%) ⬇️
src/Asset.js 68.18% <0%> (-31.82%) ⬇️
src/assets/WebManifestAsset.js 54.76% <0%> (-28.58%) ⬇️
src/utils/config.js 78.87% <0%> (-21.13%) ⬇️
src/utils/objectHash.js 84.61% <0%> (-15.39%) ⬇️
... and 55 more

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 95a6ebf...a6d6c1e. Read the comment docs.

@devongovett
Copy link
Member

I've rewritten the resolver in #850. It should be easier to disable builtins with that.

@lbguilherme
Copy link
Contributor Author

I'll rebase this once #850 is merged. Much better with the new resolver.

@devongovett
Copy link
Member

Merged the resolver.

@lpil
Copy link

lpil commented May 9, 2018

Hi all. Would it be possible to get this rebased and merged?

@lbguilherme
Copy link
Contributor Author

I don't have the time to move this forward myself right now. I'll probably only be able to rebase this PR in a month or two.

@devongovett
Copy link
Member

Updated this in #1690. Closing.

@devongovett devongovett closed this Jul 9, 2018
@Stradivario
Copy link
Contributor

@lbguilherme where it is implemented ?

From a long time ago i try and looking for solution but this option is unavailable.

I am getting

error: unknown option `--bundle-node-modules'

Any ideas ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants