-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Getting IPFS to work with ES Modules and Rollup #1927
Comments
hi @AlbertoElias thank you for this research, im trying to fix some of these problems the first step is this #1795 to streamline dependencies and reduce bundle size. but there still much to do, and some of it is related to this work you did, the end game will be to change our current webpack config with rollup and have a esm version. did you manage to understand why rollup doesn't support readable-stream ? |
Wow, that PR sure seems promising :)
That's really good to hear!
sadly, no, this is the response I got a long time ago: calvinmetcalf/rollup-plugin-node-builtins#35 (comment) which isn't very useful |
Related: I just chatted with Myles Borins who is working on ESM in Node.js and there are some big changes/improvements that will be landing soon. In particular, my complaints about variances between Node’s dynamic import method and webpack’s are going to be addressed, so it should get easier to maintain a future ESM code base for Node.js and browsers.
…-Mikeal
________________________________
From: Alberto Elias <notifications@github.com>
Sent: Wednesday, March 13, 2019 10:24 AM
To: ipfs/js-ipfs
Cc: Subscribed
Subject: Re: [ipfs/js-ipfs] Getting IPFS to work with ES Modules and Rollup (#1927)
Wow, that PR sure seems promising :)
change our current webpack config with rollup and have a esm version.
That's really good to hear!
did you manage to understand why rollup doesn't support readable-stream ?
sadly, no, this is the response I got a long time ago: calvinmetcalf/rollup-plugin-node-builtins#35 (comment)<calvinmetcalf/rollup-plugin-node-builtins#35 (comment)> which isn't very useful
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#1927 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAACQxIneQK2KSaClzNToxqtUch6wgdOks5vWTRigaJpZM4btRIg>.
|
Any timeline for this? |
I'm trying to figure if this would help a problem I've got ... someone wants to use ES6 modules (in the browser) to import our https://github.com/dweb-transports library, which currently includes IPFS via Am I correct that successfully concluding this would allow include IPFS via e.g. |
@mitra42 that used to be the case, but it looks like people have implemented proper plugins for node modules and require for rollup https://stackoverflow.com/questions/50081548/how-to-make-rollup-expand-require-statements?answertab=active#tab-top You probably won’t get tree shaking within the required modules though, since the way rollup wrote that feature isn’t all that smart and just excludes code you didn’t include in import statements. |
Does anyone know if this (using rollup) has been done successfully for IPFS. I think that if IPFS has a successfull rollup then I can either import it in my own rollup OR copy whatever workarounds the IPFS rollup uses into one layer higher up. If IPFS itself can't rollup then the chances of doing it one more level up seems unlikely ... |
Ok - I'm presuming no answer means no-one has managed to get IPFS work in ES6 modules - which is unsurprising given IPFS's dependency complexity and the challenges of these rollups. No problem, I'm going to restructure so that IPFS is included separately from our (dweb-transports) library, that way those who don't need ES6 module compatability can use a separate "require" (NodeJS) or <script> (browser) include of IPFS and those who need ES6 modules can just continue without IPFS until the IPFS community has time to figure it out. |
I haven’t tried to get this working in My recommendation is to not spend a lot of time on this right now. It’s still unclear to me how this is going to shake out in the ecosystem as Node.js support for ESM rolls out. It’s sort of surprising, but virtually all of the existing new-style modules in npm don’t actually work in Node.js’ new-style module loader without a compiler, nor do they work in browsers without a compiler. For years now, ESM has just been syntax implemented by compilers. Looking at it this way, it’s not actually very compelling, it’s just minor syntactic sugar with an easier path to tree shaking (but we actually have tree shaking for require based compilers as well). But now that native support is more widely available there’s new things we can do with ESM that we couldn’t do before, but it has to be used and implemented in a particular way that doesn’t rely entirely on a compiler. Very few people are using this syntax natively, in the browser or in Node.js. To me, the main benefit of adopting the syntax, which would be a huge undertaking, would be finding a way to more seamlessly support Node.js and Browsers, potentially without a compiler, and how to best do that is still being explored. I’m optimistic, even excited, about what this will look like, it’s just not quite figured out enough for a project of this size to try and figure out. |
Yes - the excitement does seem a bit premature :-). We've pulled IPFS out of dweb-transports anyway because of the webpack/Uglify issue ( #2411 ) it was getting too hard to support different sets of requirements around packing, so now, people using it can require it in browser or NodeJS, and eventually via ESM once ESM stabilizes enough to warrant the work, or once you can find a compiler that can roll it up into a single ESM module :-) |
less-hacky way: gulp.task('sync--ipfs--build-module', async () => {
const bundle = await rollup.rollup({
input: './node_modules/ipfs/dist/index.js',
plugins: [
rollupCommonJs(),
],
});
return await bundle.write({
sourcemap: true,
format: 'es',
dir: './src/ipfs/',
});
}); |
Still having issues with this, throwing all kinds of errors like |
@TJKoury I've not tinkered with js-ipfs in a while, does my workaround behave as needed for you? |
I thought I'd mention that since Js-IPFS got type coverage I've been using an extremely trivial shim to wrap the browser dist always included with the My project is only Snowpack and not Rollup, but Snowpack is still Rollup under the hood so I thought I'd mention it. |
It works on It now works when loaded from script. <script src="https://cdn.jsdelivr.net/npm/ipfs-core@0.5.4/dist/index.min.js"></script> const IPFS = window.IpfsCore; |
I used Minimal repo & README example here: https://github.com/DougAnderson444/ipfs-vite-svelte-kit |
I have such a problem now. |
@zababurinsv I'd suggest poking around the repo, aside from |
I looked at the project. very interesting. |
IPFS is all ESM since |
Type: Bug
Severity: Medium
Description:
So I spent quite a lot of time yesterday trying to compile js-ipfs with Rollup yesterday and here are my findings:
So, to sum up, js-ipfs ends up depending on old unmaintained dependencies that don't work well at all on the Web. They depend too much on Node, Common.JS and circular dependencies. It would be great to see js-ipfs using a more cleaned up dependency tree that's friendlier to browsers and makes it easy to use ES6 modules
Steps to reproduce the error:
index.js
rollup.config.js
The text was updated successfully, but these errors were encountered: