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

Module not found: Error: Can't resolve 'stream/web' #117

Closed
ctq123 opened this issue Sep 8, 2021 · 20 comments
Closed

Module not found: Error: Can't resolve 'stream/web' #117

ctq123 opened this issue Sep 8, 2021 · 20 comments
Labels
bug Something isn't working

Comments

@ctq123
Copy link

ctq123 commented Sep 8, 2021

Reproduction

Module not found: Error: Can't resolve 'stream/web' in '/Users/alan/project/duapp/vscode/packages/du-i18n/node_modules/fetch-blob'

image

Steps to reproduce the behavior:

  1. yarn
  2. yarn start
  3. start fail

Expected behavior

start success

Screenshots

Your Environment
vscode extension development

software version
node-fetch 3.0.0
node v14.17.3
npm 6.14.13
Operating System macos

Additional context

@ctq123
Copy link
Author

ctq123 commented Sep 8, 2021

image

@Richienb Richienb transferred this issue from node-fetch/node-fetch Sep 8, 2021
@Richienb Richienb added the bug Something isn't working label Sep 8, 2021
@jimmywarting
Copy link
Contributor

don't get why u recive a error. it's wrapped around a try/catch

@LinusU
Copy link
Member

LinusU commented Sep 8, 2021

@ctq123 I believe that this is only a warning right? When WebPack detects an import inside try/catch it just prints a warning, the bundle should still be okay to use...

@btakita
Copy link

btakita commented Sep 10, 2021

Running sapper, rollup, on AWS Lambda with version nodejs14.x.

I'm getting the error:

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'stream' imported from /path/to/apps/web/server2.js
    at packageResolve (internal/modules/esm/resolve.js:664:9)
    at moduleResolve (internal/modules/esm/resolve.js:705:18)
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:819:11)
    at Loader.resolve (internal/modules/esm/loader.js:89:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:242:28)
    at ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:73:40)
    at link (internal/modules/esm/module_job.js:72:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Rollup treats 'stream/web' as an external & hoists the require to the top of the js payload file.

The output looks something like this:

import require$$0 from 'stream/web';
// ...
if (!globalThis.ReadableStream) {
  try {
    Object.assign(globalThis, require$$0);
  } catch (error) {
		// TODO: Remove when only supporting node >= 16.5.0
    Object.assign(globalThis, ponyfill_es2018.exports);
  }
}

btakita added a commit to btakita/hbr-sapper that referenced this issue Sep 10, 2021
@jimmywarting
Copy link
Contributor

jimmywarting commented Sep 11, 2021

Hmm, i did not account for bundlers to hoist stuff like this. This kind of conditional import should really be converted to top level await + async import to match the same behavior...
Should report/suggest it to rollup

@wood1986
Copy link

wood1986 commented Sep 12, 2021

webpack should add stream/web to NodeTargetPlugin. See my PR

@ctq123
Copy link
Author

ctq123 commented Sep 15, 2021

bundle should still be okay to use.

yes,it don't affect packaging, but it's not friendly to runtime

@dominikg
Copy link

dominikg commented Sep 16, 2021

this issue prevents sveltekit from updating to node-fetch 3.0.0

I've tried looking into it yesterday and wonder about a couple of things:

  1. you are not tagging node internal modules with node: which is recommended for esm.
    Would tagging the internal modules make life easier for bundlers to detect new/unknown node internals?

  2. the package.json files of node-fetch and fetch-blob do not contain "exports" mapping or "module" fields
    they may not be strictly required, but exporting package.json is a common courtesy and using exports is recommended here too The ESM move sindresorhus/meta#15

  3. fetch-blob uses require in streams.cjs. (to avoid using top level await for node12 support i guess)
    Are there ways to avoid this direct require?

@jimmywarting
Copy link
Contributor

  1. this old issue made me belive i targeted 12.17 Support Node.js v12 #98 but the node.engine is set to 12.20. so we should be okey to use node:
  2. so far i have not had any problems with it with the way it's right now.
  3. yes it's to avoid top-level await for 12.20 support... haven't come up with any better solution yet

and lazily dynamic import

dynamic import is challenging const stream = new Blob().stream() is all sync would be different if stream returned a promise...

i have tried to best come up with a solution that don't import/depend on core node stuff. so that require can gracefully fail in Deno and Browser
like this result: https://cdn.jsdelivr.net/npm/fetch-blob/+esm
but others produce stuff like: https://jspm.dev/fetch-blob that fails totally

@benmccann
Copy link
Contributor

Here's the problematic line for reference:

Object.assign(globalThis, require('stream/web'))

@jimmywarting
Copy link
Contributor

Would anyone be able to report this as a bug to Rollup that it's hoisting the import outside of the try / catch?

Kind of already did: rollup/rollup#3621

@jimmywarting
Copy link
Contributor

I'm wondering if anyone has ideas for reproducing this for a bug report?

It could also be someones else fault... I mean the file is in a .cjs extension. (cuz it's suppose to stay in cjs format) but for some reason it's being rewritten into import require$$0 from 'stream/web';

@benmccann
Copy link
Contributor

This looks like it could be related: https://github.com/rollup/plugins/tree/master/packages/commonjs#ignoretrycatch. Vite includes @rollup/plugin-commonjs by default. Though I tried setting it to true and it didn't seem to change anything

@benmccann
Copy link
Contributor

Actually, that option did fix the error for me. I just set it in the wrong place the first time around. Here's a working version: sveltejs/kit#2422

I think the default value for this option should be changed in @rollup/plugin-commonjs to avoid this issue: rollup/plugins#1004

@jimmywarting
Copy link
Contributor

jimmywarting commented Sep 27, 2021

Thank you @benmccann for finding the root cause and reporting issues

@benmccann
Copy link
Contributor

@rollup/plugin-commonjs 21.0.0 is out now, so hopefully this should be fixed. I've sent PRs to upgrade SvelteKit sveltejs/kit#2539 and Vite vitejs/vite#5173

@benmccann
Copy link
Contributor

Vite 2.6.3 and SvelteKit 1.0.0-next.180 have been released and use the fixed version of @rollup/plugin-commonjs

@jimmywarting
Copy link
Contributor

good to hear, dose it work okey now to include fetch-blob?

@benmccann
Copy link
Contributor

Yes

btakita added a commit to btakita/hbr-sapper that referenced this issue Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants