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

vite does not handle "browser" field in package.json properly, when using package 'tapable' #7576

Closed
7 tasks done
depressedX opened this issue Apr 2, 2022 · 9 comments · Fixed by #10314
Closed
7 tasks done
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@depressedX
Copy link

Describe the bug

Just install npm package tapable, and import it in main.js

import { SyncHook } from 'tapable';
console.log('load Hook', SyncHook);

Then the console will print an error:
image

Then I digged into node_modules/tapable/package.json, and it use browser field for different platforms.

image

and package util is imported in tapable/lib/Hook.js Line 7
image

So In expected result, I will get the file util-browser involved, which described in browser. But actually the pre-bundled code just resolved this into some compatibility code.

image

So how can I solve this?

Reproduction

https://stackblitz.com/edit/vitejs-vite-6grfch?file=main.js

System Info

System:
    OS: macOS 11.4
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 105.61 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.19.0 - ~/.nvm/versions/node/v14.19.0/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v14.19.0/bin/yarn
    npm: 6.14.16 - ~/.nvm/versions/node/v14.19.0/bin/npm
  Browsers:
    Safari: 14.1.1
  npmPackages:
    @vitejs/plugin-legacy: ^1.7.1 => 1.8.0
    vite: ^2.8.3 => 2.9.1

Used Package Manager

yarn

Logs

No response

Validations

@depressedX
Copy link
Author

seems like vite got wrong id from esbuild onResolvehook.

https://github.com/vitejs/vite/blob/main/packages/vite/src/node/optimizer/esbuildDepPlugin.ts#L109

image

image

The id should be some sort of ./lib/util-browser.js, I guess.

@depressedX
Copy link
Author

depressedX commented Apr 2, 2022

Another question:
When you install npm package util, which is an polyfill implement for node internal module util, vite will get this package bundled(instead of giving the 'browser external hint').
Well I think it's better treating these polyfill implements for node internal module as the original node internal modules

@AnderbergE
Copy link

I believe I have a similar problem when using the package websocket-stream (directly, or through aws-iot-device-sdk).
The "browser" field in websocket-stream is disregarded, which means that instead of using the browser's WebSocket, a Node-dependant version is bundled.

@AnderbergE
Copy link

Does Vite not care about the browser field at all maybe?
See this reproduction: https://stackblitz.com/edit/vite-qes2gf?file=package.json
If I use webpack the lodash module import is replaced by the content in the file ./src/mylodash.js. In Vite it is not.

@prichodko
Copy link

Run into a similar issue, when package defines inside package.json a dependency which should be ignored when bundling like:

{
   "browser": {
        "dependency": false
   }
}

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Jun 22, 2022
@tuul-wq
Copy link

tuul-wq commented Jul 2, 2022

I'm trying to build Vite project with a @polkadot/rpc-provider package which has @substrate/smoldot-light deep down below.
Looks like the same problem with browser field. Webpack 5 handles it properly.

@substrate/smoldot-light/package.json

"browser": {
  "./dist/compat/index.js": "./dist/compat/index-browser-overwrite.js",
  "./dist/worker/spawn.js": "./dist/worker/spawn-browser-overwrite.js"
},
Console:

[vite:worker-import-meta-url] 'parentPort' is not exported by __vite-browser-external, imported by node_modules/@substrate/smoldot-light/dist/compat/index.js
file: /Users/yaroslav/Documents/Stuff/Projects/vite-number-conversion/node_modules/@substrate/smoldot-light/dist/worker/spawn-browser-overwrite.js:17:9
15: //
16: // A rule in the `package.json` overrides it with `index-browser-override.js` when in a browser.
17: import { parentPort } from 'node:worker_threads';
             ^
18: import { hrtime } from 'node:process';
19: import { createConnection as nodeCreateConnection } from 'node:net';

@asiaziola
Copy link

asiaziola commented Jul 19, 2022

Hey, similair issue. I'm using dependency which sets couple of modules to false in browser field. However, they are still included in the build

Screenshot 2022-07-19 at 10 31 38

Screenshot 2022-07-19 at 10 30 57

@lemoyim
Copy link

lemoyim commented Aug 2, 2022

I have similar problem with xrpl.js which uses ws package for Node and specifies the wrapper around browser's native websocket API in their package.json. The browser build dismisses this and just tries to use ws, which throws error.

https://github.com/XRPLF/xrpl.js/blob/main/packages/xrpl/package.json#L20-L22

A workaround of putting an alias for it manually works (in your vite.config.js):

resolve: {
		alias: {
			// your other aliases
                        // ...
			ws: './node_modules/xrpl/dist/npm/client/WSWrapper.js'
		}
	},

Seems like there is a PR for it #8709
I am, unfortunately, don't understand vite's internals well enough to help review it.

@zizifn
Copy link

zizifn commented Oct 8, 2022

Does this fix is in "vite": "^3.2.0-beta.0",? if so, I don't think this is fixed, please check the test repo compare to webpack with more details about the test case.
https://github.com/zizifn/vite-browser-test

@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.