Automatically exclude node-specific packages from browser bundlers

[fregante]

As discussed in #561 (comment)

I'm opening a specific issue to this because the other one was due to a now-resolved Parcel issue.

In short: It'd be great to automatically offer a browser-only path to ensure:

• browser builds don't include unnecessary path and stream packages
• bundlers such as webpack v5 don't trip over node-only packages (without requiring extensive configuration)

[ljharb]

I'm not sure what you're asking for here.

The only correct thing for a node module bundler to do is provide browser polyfills for node core modules (when feasible), including per-module "globals". path and stream both work fine in the browser, so it's perfectly fine for tape (or any package) to depend on them unconditionally.

fs, of course, can't work in the browser, so it should be possible for fs to be substituted with an empty object, and the code should still work - which is what browserify and pre-broken webpack versions have always done. #565 may make it slightly easier for bundlers to handle fs, and that's great.

Webpack 5's choice means that every single user of it can't avoid extensive configuration, and it's not up to individual package authors to fix that problem - it's up to webpack's authors.

[fregante]

I'm not sure what you're asking for here.

To provide browser-specific entry points that don't make use of Node-specific tools.

it's perfectly fine for tape (or any package) to depend on them unconditionally

Yes and no. While they work in the browser, they still need to be bundled (even with browserify), increasing the bundle size (and in this case, configuration).

However, in tape’s case:

• stream seems to be a fundamental part of tape so it can't be dropped
• bundle size is not a priority, since it's run locally

This means:

• configuration for webpack 5 is still needed

One possible improvement would be to exclude path somehow since it's currently only used here (in the non-bin entry point):

https://github.com/substack/tape/blob/a7ca7a308269bc3a250170441553d0321e0d8044/lib/test.js#L317

But it's not a big deal, so I'll close this issue.

[ljharb]

Bundling is a requirement of modern web dev; it's not practical to avoid using one. Bundle size shouldn't matter for a testing tool, even when run in a browser.

path.sep handles Windows cleanly, so i'm not sure how to exclude path there.

[fregante]

Bundling is a requirement of modern web dev; it's not practical to avoid using one

Where did I say otherwise?

---

Bundle size shouldn't matter for a testing tool

I said that 👇

• bundle size is not a priority, since it's run locally

---

path.sep handles Windows cleanly, so i'm not sure how to exclude path there.

If you map path to false like we did for fs, you can then detect the lack of it and just avoid using it the whole code block. There are no paths in the browser (but I may be wrong, I'm not entirely sure about that file is doing) or probably it deals with URLs, why only have one separator.

But then again:

• configuration for webpack 5 is still needed

So

it's not a big deal,

[ljharb]

ah, i misunderstood "they still need to be bundled", my bad.

it's not running locally that makes bundle size irrelevant, it's that "it's not for production, it's for dev". Bundle size only matters for production web code.

…

that's a good point. if __dirname and/or path are not present, we could skip https://github.com/substack/tape/blob/a7ca7a308269bc3a250170441553d0321e0d8044/lib/test.js#L366, and if __dirname is not present (ie, in broken bundlers), we could skip that entire logic.

I'd be willing to accept a PR for that, since the only cost paid by those using working bundlers is a pretty trivial if check.

[fregante]

I'd be willing to accept a PR for that

Thanks! However this would need some testing to ensure it works correctly and it's probably not worth the effort at this point, especially since I'm not entirely familiar with that piece of code.