Disabling native node modules with browser field not working

[SamyPesse]

esbuild supports the browser field in package.json, but it doesn't seem to support it for native node modules (such as fs, etc)

I'm having the issue while building a project with the dependency @apidevtools/json-schema-ref-parser which explicitly set fs to false

node_modules/@apidevtools/json-schema-ref-parser/lib/util/url.js:23:24: error: Could not resolve "url"
node_modules/@apidevtools/json-schema-ref-parser/lib/util/url.js:24:26: error: Could not resolve "url"
node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers/http.js:3:21: error: Could not resolve "http"
node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers/http.js:4:22: error: Could not resolve "https"
node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers/file.js:2:19: error: Could not resolve "fs"

The error concerning url, http, https are making sense because they are not being listed in the browser field (and can be added as dependencies in the parent project from Yarn). But the one for fs doesn't make sense.

I've seen integration tests for the browser field, but I feel like one is missing for native node modules.

The issue seems to come from the loadNodeModule call when the module is set to false in browser.

esbuild/internal/resolver/resolver.go

Lines 201 to 206 in 99dc3f7

	if remapped == nil {
	// "browser": {"module": false}
	if absolute, ok := r.loadNodeModules(importPath, sourceDirInfo); ok {
	return absolute, ResolveDisabled
	} else {
	return "", ResolveMissing

It looks like loadNodeModule should have a list of native node modules to consider as existing. Or if the value in browser is false, it should just ignore the module.

[evanw]

I think I understand what's going on. This isn't related to fs being a node builtin module. This happens with any module identifier that doesn't correspond to something on disk (e.g. also with a module called not-fs).

The problem is that esbuild still maps this module to an absolute path even though it's disabled. That way all calls to require('fs') return the same object, which is how CommonJS is supposed to work. This is why the disabling fails if the module can't be found on disk.

I'll think of a way to solve this. Perhaps a good way to solve this is to just have every require() call return a different empty object. And thanks for the test case :)

[SamyPesse]

@evanw quick questions because I'm wondering if the behaviour should not be extended.

Using the module https://github.com/APIDevTools/json-schema-ref-parser is not possible with esbuild, because this module imports http and https that cannot be installed from NPM. Webpack keeps an internal mapping for module (https://github.com/webpack/node-libs-browser/blob/master/index.js) and use https-browserify and stream-http instead, which I don't think is something esbuild should choose.

A solution with other bundlers is to use the browser field in the parent package.json:

package.json    <- For esbuild: specify mapping with browser field here doesn't work
somecode.ts
node_modules/
   @apidevtools/
         json-schema-ref-parser/
              package.json  <- this one only contains "fs": false, nothing linked to http/https

But esbuild only uses the browser field of the module itself, not from the project.

I'm gonna try with a typescript paths, but I think browser mapping should be extended by all packages in the tree.