Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When upgrading from 4.3.0 to 4.4.0, using esbuild --bundle to build will report an error Could not resolve "stream" #516

Closed
leizongmin opened this issue May 10, 2023 · 5 comments · Fixed by #520

Comments

@leizongmin
Copy link

This is an example file test.js:

require('readable-stream');

Run the following commands (esbuild@0.17.12):

esbuild --bundle test.js

It outputs:

✘ [ERROR] Could not resolve "stream"

    node_modules/.pnpm/readable-stream@4.4.0/node_modules/readable-stream/lib/stream/promises.js:7:8:
      7 │ require('stream')
        ╵         ~~~~~~~~

  The package "stream" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "--platform=node" to do that, which will remove this error.

1 error

If I use version 4.3.0, it will output correctly.

@kanongil
Copy link

This seems like another instance of the issue in #513, though this exposes it as a regression!

@leizongmin
Copy link
Author

The problem was introduced from this commit b345071#diff-241fa22640048b6a147dec92f1ebf253043a1c4c342c7e52402112eb3c58f8fbR7

require('stream')

I have another question: why require this module, but not use any properties or methods of stream?

@leizongmin leizongmin changed the title When upgrading from 4.3.0 to 4.4.0, using esbuild --bundle to build will report an error Could not resolve "stream" When upgrading from 4.3.0 to 4.4.0, using esbuild --bundle to build will report an error Could not resolve "stream" May 10, 2023
Windvis added a commit to lblod/frontend-worship-decisions that referenced this issue May 11, 2023
Once the issue is resolved we can remove the override again.

More information: nodejs/readable-stream#516
@jeswr
Copy link
Contributor

jeswr commented May 12, 2023

In order to avoid such regressions in the future can I suggest adding a test to this library which webpacks

import-all-test.js

import * as all from 'readable-stream'

using the config

webpack.config.js

const path = require("path");
module.exports = {
  mode: "development",
  entry: "./integration-tests/import-all-test.js",
  output: {
    path: path.resolve(__dirname, "integration-tests", "dist"),
    filename: "import-all-test.js",
  },
};

The test would just be running npm run test:build where that script is defined as "test:build": "webpack"

@mcollina
Copy link
Member

Would you like to send a PR to fix this (adjust the build scripts) or just with the test?

@jeswr
Copy link
Contributor

jeswr commented May 13, 2023

#517 should cover the main point - I will see if I can make that config more restrictive after the issue is patched.

rubensworks added a commit to comunica/comunica-feature-link-traversal that referenced this issue May 24, 2023
The latest version has a webpack issue at the moment
nodejs/readable-stream#516
rubensworks added a commit to comunica/comunica-feature-link-traversal that referenced this issue May 25, 2023
The latest version has a webpack issue at the moment
nodejs/readable-stream#516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants