Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Parcel fails to build js-ipfs (and many other things) #1344

Closed
olizilla opened this issue May 8, 2018 · 15 comments
Closed

Parcel fails to build js-ipfs (and many other things) #1344

olizilla opened this issue May 8, 2018 · 15 comments

Comments

@olizilla
Copy link
Member

olizilla commented May 8, 2018

  • Version: 0.28.2
  • Platform: All
  • Subsystem: All

Type:

Bug / Warning to others

Severity:

High

Description:

It's not currently possible use parcel to create production builds of apps that depend on js-ipfs. This is due to an issue in parcel, not js-ipfs.

The parcel build command tries to dedupe modules to reduce bundle size, but an error in the logic is leading it to incorrectly dedupe ipld-git and ipld-dag-cbor to the same module, which causes an error in js-ipld, when we try to add two identical resolvers. Ipld correctly throws an error in that case. The error is that the parcel build has resolved require('ipld-git') to the same module as require('ipld-dag-cbor')

grep ipld-git dist/app*.js

dist/app.79643623.js:},{"ipfs-block":111,"pull-stream":124,"cids":38,"async/doUntil":125,"ipfs-repo":109,"ipfs-block-service":28,"path":96,"pull-defer":129,"pull-traverse":130,"async/map":126,"async/series":127,"async/waterfall":128,"interface-datastore":112,"ipld-dag-pb":33,"ipld-dag-cbor":31,"ipld-git":31,"ipld-bitcoin":114,"ipld-ethereum":115,"ipld-raw":116,"ipld-zcash":114}],337:[function(require,module,exports) {

Notice how ipld-git and ipld-dag-cbor map to the same integer. The same bad dedupe has happened to ipld-bitcoin and pld-zcash as well, but we hit the ipld-git error first.

This issue is here as warning to others.

The fix for this issue is in parcel-bundler/parcel#1011

Workarounds

  • Use https://github.com/tableflip/window.ipfs-fallback which will pull in js-ipfs from unpkg if needed or use window.ipfs where available.
  • Use a pre-built version of js-ipfs from unpkg directly.
  • Use browserify or webpack
  • Use parcel, but don't use the build command. Not ideal as your bundle will be unoptimised.

Steps to reproduce the error:

You can see it on the circuit-relaying example in this repo:

cd examples/circuit-relaying
npm i

# Create a production build with parcel
npm run build
# Serve the app
npx http-server@latest dist

Loading the app in a browser shows the following error:

index.js:48 Uncaught Error: dag-cboralready supported
    at Object.IPLDResolver.support.add (index.js:48)
    at new IPLDResolver (index.js:72)
    at new IPFS (index.js:80)
    at Object.parcelRequire.6.ipfs (app.js:29)
    at newRequire (rsa-utils.js:73)
    at parcelRequire.28 (index.js:75)
    at index.js:101

screenshot 2018-05-08 17 56 21

@dryajov
Copy link
Member

dryajov commented May 8, 2018

Thank you for troubleshooting this 👍

I've observed some intermittent issues with parcel when developing the example, but wasn't able to reliably reproduce them. They were mostly performance issues that would manifest after running the example for prolonged periods of time. However, this seems to be a different issue, I wonder if it was introduced recently or if it exists in previous versions as well?

@Mr0grog
Copy link
Contributor

Mr0grog commented May 8, 2018

Yeah, was going to say the same: could we fix this by locking in the Parcel version a little tighter? It definitely worked for me while reviewing the PR. (Unless what actually happened is: a change in js-ipfs or its submodules now triggers the bug when it didn’t before, as opposed to a change in Parcel introducing the bug.)

@Mr0grog
Copy link
Contributor

Mr0grog commented May 8, 2018

As a separate issue, it might be useful to make sure all the examples use the same bundling system (whether Parcel or something else). Less confusion about different tools across the examples and less to download and, ideally, fewer latent issues because they’ll show up everywhere instead in just one example.

@dryajov
Copy link
Member

dryajov commented May 8, 2018

I can change it to browserify if that is preferred.

@Mr0grog
Copy link
Contributor

Mr0grog commented May 8, 2018

How about seeing if pinning directly to 1.6.2 works and make separate issue for unifying bundlers? I have a feeling the choice of what bundler might be contentious 😬

@olizilla
Copy link
Member Author

olizilla commented May 8, 2018

+1 for pinning.

There is value in having the examples show a variety of different packaging methods, and parcel is useful for getting started quickly. It sounds like the PR to fix it will be mergable this week.

@olizilla
Copy link
Member Author

olizilla commented May 9, 2018

To dedupe, Parcel uses the source file as a key in a Map. identical files are deduped, regardless of where in the tree they came from.

Our ipld resolver modules follow a pattern that means they often have identical index.js files, like

so for a production build, those two index.js files get deduped, and ipld-dag-cbor is returned for both requires.

@olizilla
Copy link
Member Author

olizilla commented May 9, 2018

@gabrielpoca I just spotted in the #ipfs irc logs that you hit the same error a while back trying to bundle js-ipfs with parcel. Good news (a little late, but better late than never!) there is a PR to fix this on parcel over at parcel-bundler/parcel#1011

@gabrielpoca
Copy link

Hey @olizilla I wasnt expecting to be notified about something I asked on irc, thank you! This was awsome!

@olizilla
Copy link
Member Author

olizilla commented May 9, 2018

@dryajov @Mr0grog alas it looks like all versions of parcel exhibit this problem, as the dedupe logic is in v1.0.1 parcel-bundler/parcel@96bf882#diff-b61d675f2dd7341e1925f9d7e7767689

Also of note, the example here is still totally useable, npm start still works. This issue only shows up when you use parcel build to generate the optimised production build.

@Mr0grog
Copy link
Contributor

Mr0grog commented May 9, 2018

npm start still works. This issue only shows up when you use parcel build to generate the optimised production build.

Ah! I had completely missed that part. No wonder I didn’t see the issue — I don’t think I ever tried parcel build.

@dryajov
Copy link
Member

dryajov commented May 9, 2018

Thanks @olizilla! That makes sense.

Tho I agree with @olizilla on showing different bundlers in action, perhaps, the best way of doing that would be through docs, and smaller examples that would demonstrate specifically the bundler functionality and how to use it with js-ipfs, and leaving the larger examples working with browserify (or whatever bundler we decide).

@olizilla
Copy link
Member Author

olizilla commented Jul 3, 2018

Fix is merged to parcel master, just waiting for the next release now.
parcel-bundler/parcel#1011 (comment)

@alanshaw
Copy link
Member

alanshaw commented Jul 7, 2018

For anyone else wondering if it was released yet - when parcel version is >1.9.4 this will be fixed

@alanshaw
Copy link
Member

It is released, anyone available to test?

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

No branches or pull requests

6 participants