Add experimental support to polyfill/shim builtin modules

[FredKSchott]

I'm creating this PR verrrrry tentatively, since I'm worried it will give the false impression that any Node dependency is fine with this option, when really it's a sign that some dependency in your tree probably isn't meant for the web.

I may end up restricting this to just a set of packages that we feel comfortable with, like "path"/"fs"/"url" and continue to fail if more complex packages like "fs"/"http"/"crypto" are encountered.

[FredKSchott]

It's been about a month and I haven't heard any complaints/comments/asks for this, so I might just leave this in purgatory. If you run into a valid use case where this is needed, lmk.

[zaygraveyard]

I think I'm a little late to the game but I have a use case 😅
I'm trying the "pika" way:

1. prepare dependencies when they change
2. build/bundle for deployement/production
3. use a modern browser for developement

So I'm using:

• @pika/web for the depenencies (with its awesome automatically generated import map)
• flow comments for type checking
• developit/htm for JSX in the browser along with preact
• zaygraveyard/flow-comments a tool I built to help with JSX to HTM conversions and Prettier's buggy/non-existant support for flow comments (I'm planing on replacing this with a more modular and plugable version, something like codemod-js/codemod but using Rich-Harris/magic-string for the actual modifications)
• guybedford/es-modules-shims for module/imports related support in non-supporting browsers
• rollup and all the reset for production bundling (as module and nomodule)
• theintern/intern for testing with a custom loader to support ES modules via es-modules-shims (https://gist.github.com/zaygraveyard/ae6cc847663fdc3898e51303f1c5a66d)

I wanted to add power assert for better assertions in the tests
And found that all the instrumentation packages offered by power assert (like espower) were not very browser friendly (they required node builtin modules) but they work just fine in the browser once bundled with Rollup and rollup-plugin-node-builtins

So now I bundle it manually, place it web_modules and add an entry in the import map JSON.

But if this PR gets merged, it will all be handled magically with pika install

[FredKSchott]

/cc @dmnsgn who started this convo on our discussion board as well: https://www.pika.dev/npm/snowpack/discuss/52

So after looking back into this, it looks like Rollup's node builtin story is pretty weak. There's an unofficial plugin for it, but the author is absent. The Rollup team tried to bring it into their org, but realized it was unlicensed. There's a fork with some issues fixed that this PR uses, but that's also about a year out of date now. So it's a bit of a mess.

While I was searching though, I also came across this: https://github.com/jvilk/BrowserFS

"BrowserFS contains shim modules for fs, buffer, path, and process"

I think those are the most commonly used packages needing shim, and I bet we could get away with just supporting those at least to start. BrowserFS is ~3 packages total, while the rollup plugin was going to add ~80.

[FredKSchott]

Hmm, BrowserFS is a bit weird too, due to the fact that it's heavily using UMD it's expecting a lot of heavy lifting to be done by a window.BrowserFS global, which is never imported but referenced everywhere. More investigating needed

[Jack-Works]

:sad In my use case the built in module crypto is required

[FredKSchott]

Your package uses crypto on the frontend? Can you send a link?

[Jack-Works]

Your package uses crypto on the frontend? Can you send a link?

DimensionDev/Maskbook#372

We're using bip39, wallet.ts, web3 and so on.. to do the blockchain stuff.

[dy]

My couple of cents.
When I found that snowpack actually supports import-maps, I decided to ditch parcel (it generates suboptimal builds and introduces errors #3575) and give snowpack a try with our new website.

And it feels great.
But some things are yet blockers for smooth workflow. This is one of that.
I faced trouble snowpacking jsonwebtoken, that we use for our website.
Related reports #62, #52. And there's going to be more.

Too many browser packages use default node builtins. To estimate - have a look at direct dependents of buffer, stream, path, util, events etc, browserify shims builtins with these packages (here). Consider also the amount of indirect dependents. Many orgs (used to) take browserify as normal practice: https://github.com/scijs, https://github.com/stackgl, https://github.com/gl-vis etc.

The history of browserifying things has long roots. There are solutions for almost everything, even fs - eg. brfs inlines file content in bundle, so for user that feels seamless. The main drawback of browserified builtins is bundle size. For example, crypto is 230kb minified.
I think required node "builtins" could be considered as usual npm deps, not listed in package.json, but "installed" implicitly.

[dy]

browserify has --no-builtins flag for that purpose. https://github.com/browserify/browserify#usage

[FredKSchott]

(Looks like your comment was marked as "off topic" accidentally. Since I asked for feedback in the first comment above, that feedback was perfectly fine. Sorry about that! /cc @monchi @DangoDev lets reserve comment hiding for spam/abusive comments only.)

+1, at this point I'm on board with adding this feature as an opt-in. It's almost always a better option to find a better, more web-friendly dependency that won't bloat your app like the 230kb crypto polyfill will, but I understand that a better dep may not always exist.

The major issue, though, is that the only good plugin that does this for Rollup is 3rd party, and has been abandoned (~3 years without a push, and lots of "outdated dependency" bugs). I've just posted to the Rollup team to see if they're still interested in building a new, official plugin for this now that they have their "official plugins" repo. If you are at all interested in helping build that, both teams would be really thankful!

rollup/plugins#51

[FredKSchott]

Update: we may support this via user-provided plugins instead: https://www.pika.dev/npm/snowpack/discuss/79

[FredKSchott]

Closing in favor of #79 (improved warning here: 41a1c45)