google / playground-elements

Serverless coding environments for the web.
BSD 3-Clause "New" or "Revised" License
551 stars 55 forks source link

Alias/ignoring node modules or node specific files similar to browser field in package.json #286

Open jpoehnelt opened 2 years ago

jpoehnelt commented 2 years ago

I haven't looked through the full dependency graph for this but am running into:

playground-typescript-worker.js:37 Error fetching fs@0.0.1-security/index.js from CDN: CDN HTTP 404 error playground-typescript-worker.js:37 Error fetching child_process@1.0.2/index.js from CDN: CDN HTTP 404 error

with

import * as deckgl from 'deck.gl';

demo

Happens in 0.14.8 and 0.15.1.

This is probably fixable upstream? Not sure where though.

aomarks commented 2 years ago

fs and child_process are both Node built-in modules, so it seems like some module in that import graph is assuming it's in a Node context, instead of a Web context, and trying to import those built-ins.

One of the culprits is this file: https://unpkg.com/browse/@loaders.gl/worker-utils@3.1.8/dist/esm/lib/process-utils/process-utils.js

It seems like deck.gl is supposed to work on the web, so I'm surprised that this module is in its import graph.

jpoehnelt commented 2 years ago

Seems some bundlers such as webpack provide handling for these cases. https://webpack.js.org/configuration/node/#node, with automatic polyfills or just returning an empty module.

{  
  node: {
    fs: 'empty'
  }
}

Similar with parcel: https://parceljs.org/features/node-emulation/#polyfilling-%26-excluding-builtin-node-modules

When your code, or more likely a dependency, imports builtin Node modules such as crypto, fs or process, Parcel will automatically use one of the following polyfills. If no polyfill is available, then an empty module will be used instead.

jpoehnelt commented 2 years ago

Seems like a workaround would be to use the importMap to point at a local, empty module or polyfill if available.

aomarks commented 2 years ago

Seems some bundlers such as webpack provide handling for these cases. https://webpack.js.org/configuration/node/#node, with automatic polyfills or just returning an empty module.

Hmm, will have to think about whether we want to support this. I feel a little reluctant to add complexity to the bundling system to work around libraries that are not intrinsically web compatible. IMO libraries that want to support the web should simply not publish modules that depend on Node imports; they should have a different entry point, or a different package, for the Node context. If the feature that depends on a Node API isn't required and can simply be blanked out, then it doesn't seem like it should be in the main import graph.

Seems like a workaround would be to use the importMap to point at a local, empty module or polyfill if available.

That could work! Let me know if you get it to work.

jpoehnelt commented 2 years ago

I was able to get this to work with importMap.imports. In the future, might want to consider support for the browser field of the package.json in the form of:

browser: {
  fs: false,
  '../some/path': false<
}

(This is the other usage not the compiled js entry for browsers that is in the NPM docs).

jpoehnelt commented 2 years ago

This seems to be a never ending chase down a rabbit hole that probably won't end until I recreate the behavior in parcel/webpack/etc.