nodejs / readable-stream

Node-core streams for userland
https://nodejs.org/api/stream.html
Other
1.03k stars 228 forks source link

Typo in importing `process` in multiple files in `lib/internal/streams` #526

Open ankitzm opened 1 year ago

ankitzm commented 1 year ago

I was building an app where I imported @toruslabs/base-controllers. The @toruslabs/base-controllers have readable-stream as a dev-dependency as listed in the package.json file. image

My node version: v18.14.1

Problem

When I ran my code, it threw the below error. Clearly, the error was because it couldn't find any process/ module as there is no such thing. image

The error was fixed when I changed all the process/ to process. As I was checking through the readable-stream repository, I found multiple typos in the process module in the repo. This might be breaking a lot of apps and I think this can be easily fixed with a single PR.

Solution

The typo needs to be fixed in these files 👇 image

I would love to work on the issue, let me know if I missed something. Thank You

rluvaton commented 1 year ago

looking at the history it looks like this was intended:

vweevers commented 1 year ago

It's an old trick to tell Node.js (and bundlers that follow the same require() algorithm) to load an npm package instead of a core module.

Might be an issue with Vite specifically. We don't directly test Vite (in dev mode) but we do test Rollup here (which is what Vite uses in production mode):

https://github.com/nodejs/readable-stream/blob/268229d67620d092ea4d64de5416f55997eadbaa/test/browser/runner-prepare.mjs#L94-L97

ankitzm commented 1 year ago

Ohh, thanks for reviewing @rluvaton and @vweevers. Will try it using vite in production mode and update here. Thanks.

BryanWallin commented 5 months ago

I'm seeing this issue when trying to use archiver (which includes this project under the hood) when running a vanilla Node.js script on version 20.2.0:

Error: Cannot find module 'process/'
Require stack:
- C:\Users\user\Documents\GitHub\project\packages\scripts\zip-lambdas\node_modules\readable-stream\lib\internal\streams\end-of-stream.js
- C:\Users\user\Documents\GitHub\project\packages\scripts\zip-lambdas\node_modules\readable-stream\lib\internal\streams\operators.js
- C:\Users\user\Documents\GitHub\project\packages\scripts\zip-lambdas\node_modules\readable-stream\lib\stream.js
- C:\Users\user\Documents\GitHub\project\packages\scripts\zip-lambdas\node_modules\readable-stream\lib\ours\index.js
- C:\Users\user\Documents\GitHub\project\packages\scripts\zip-lambdas\node_modules\archiver-utils\index.js

It appears the appended / no longer works in Node.js 20? This is our usage of it:

import archiver from 'archiver';
MMesch commented 5 months ago

I ran into the same problem when working on an Obsidian plugin. It builds fine but crashes at runtime in the Obsidian electron desktop app because it can't find process/. I'm using esbuild.

https://forum.obsidian.md/t/cant-enable-plugin-depending-on-readable-stream/84235

Not sure how to work around this yet. Looking forward to ideas.

jonluca commented 5 months ago

Adding in process using yarn add process worked for now with esbuild, but it feels like this is going to break.