tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

"Error: The _read() method is not implemented" when used in the browser #561

Closed fregante closed 2 years ago

fregante commented 3 years ago

I'm unable to run tape in the browser with a simple bundle via Parcel. I receive this error at the first test:

VM30:1 Uncaught Error: The _read() method is not implemented
    at new o (<anonymous>:8:6099)
    at x.i.register.x._read (<anonymous>:1:10516)
    at x.i.register.x.read (<anonymous>:1:10218)
    at U (<anonymous>:1:8934)
    at x.<anonymous> (<anonymous>:1:11450)
    at x.i.register.a.emit (<anonymous>:1:19390)
    at x.s.resume (<anonymous>:1:3378)
    at x.To.r.resume (<anonymous>:8:84632)
    at x.i.register.x.on (<anonymous>:1:12975)
    at x.i.register.x.pipe (<anonymous>:1:11939)

Repro

  1. Create a test.js file:
import test from 'tape';

test('test using promises', t => {
  t.pass();
});
  1. Run these commands
echo '{}' > package.json
npm i tape parcel
npx parcel build test.js
  1. Copy the content of dist/main.js and execute it in the browser, like in the console.

Screenshot

Screen Shot
fregante commented 3 years ago

For the record it works if I import https://github.com/martinheidegger/fresh-tape but I'd rather not use forks since tape’s line is

tap-producing test harness for node and browsers

ljharb commented 3 years ago

Does it work with browserify or webpack < 5?

Parcel might be failing to polyfill node streams.

fregante commented 3 years ago

Webpack 4 doesn't bundle at all without custom configuration:

❯ webpack ./test.js
Hash: 79ea5ce9a7829ce029a4
Version: webpack 4.46.0
Time: 1569ms
Built at: 09/01/2021 12:54:18 AM
  Asset     Size  Chunks  Chunk Names
main.js  125 KiB       0  main
Entrypoint main = main.js
  [0] ./node_modules/process/browser.js 5.29 KiB {0} [built]
  [1] ./node_modules/call-bind/callBound.js 413 bytes {0} [built]
  [4] ./node_modules/inherits/inherits_browser.js 753 bytes {0} [built]
  [8] ./node_modules/events/events.js 14.5 KiB {0} [built]
 [11] ./node_modules/through/index.js 2.56 KiB {0} [built]
 [13] ./node_modules/timers-browserify/main.js 1.97 KiB {0} [built]
 [15] ./node_modules/defined/index.js 150 bytes {0} [built]
 [23] ./node_modules/has/src/index.js 129 bytes {0} [built]
 [24] ./node_modules/object-inspect/index.js 16 KiB {0} [built]
 [49] ./node_modules/tape/index.js 4.62 KiB {0} [built]
 [50] multi ./test.js 28 bytes {0} [built]
 [51] ./test.js 77 bytes {0} [built]
 [52] ./node_modules/tape/lib/default_stream.js 968 bytes {0} [built]
 [67] ./node_modules/tape/lib/test.js 21.6 KiB {0} [built]
[101] ./node_modules/tape/lib/results.js 6.52 KiB {0} [built]
    + 88 hidden modules

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/

ERROR in ./node_modules/tape/lib/default_stream.js
Module not found: Error: Can't resolve 'fs' in '/Users/rico/Dev/a/node_modules/tape/lib'
 @ ./node_modules/tape/lib/default_stream.js 4:9-22
 @ ./node_modules/tape/index.js
 @ ./test.js
 @ multi ./test.js
fregante commented 3 years ago

But it does work with this webpack config:

module.exports = {
    node: {
        fs: 'empty'
    }
}
ljharb commented 3 years ago

oh hm, that seems like something that should have been surfaced many years ago, and that I think can be fixed in tape.

ljharb commented 3 years ago

Actually, hmm - browserify automatically makes fs an empty object; i'm pretty confident older versions of webpack do as well. Maybe webpack 3 handles it?

ljharb commented 3 years ago

@fregante in parcel and webpack 4+, if you add "browser": { "fs": false } to tape's package.json, does that address the issue?

fregante commented 3 years ago

For the record webpack 5 also works… with lotsa configuration:

const webpack = require("webpack");
module.exports = {
  resolve: {
    fallback: {
      fs: false,
      path: require.resolve("path-browserify"),
      stream: require.resolve("stream-browserify")
    }
  },

  plugins: [
    new webpack.ProvidePlugin({
      process: "process/browser"
    })
  ]
};

Generally speaking, would it be possible to make tape more browser friendly by default? For example by having a "browser" entry point that doesn't make use of some features.

ljharb commented 3 years ago

"browser-friendly" is a property of the bundler in this case.

So, yes, tape can probably make changes that paper over the user-hostile choices some bundlers make. https://github.com/substack/tape/issues/561#issuecomment-909469015 is an easy one, if that works.

fregante commented 3 years ago

"browser-friendly" is a property of the bundler in this case.

Sure, Tape can't run in the browser without modifications from a bundler, but surely defaulting to fs and path doesn't exactly make this module browser-friendly either.

browserify’s intent was specifically to run Node’s code in the browser and it was great at that, but bundlers nowadays have a different approach to Node support (see Rollup, Webpack 5, esbuild, …) so fs/path/stream are hurdles in that context.

ljharb commented 3 years ago

Sure, and if there's an easy way to fix it that doesn't impose a cost on those using working bundlers, I'm all for it.

Can you give https://github.com/substack/tape/issues/561#issuecomment-909469015 a shot and confirm that will help?

fregante commented 2 years ago

Yes, adding that line to node_modules/tape/package.json helps!

I opened a specific issue regarding path and stream, since this issue was actually due to Parcel and unrelated to Tape (even though it could be avoided altogether with https://github.com/substack/tape/issues/564)