Resolve builtins with --no-browser-field, fixes #1654

[goto-bus-stop]

Currently when browserField: false or --no-browser-field is used, builtin modules like path and stream are not resolved correctly. They go through the resolve module, which simply returns the module name for core modules. Then module-deps tries to read the file named path which doesn't exist (usually).

With this patch, builtin modules are checked before calling into the resolve module.

TODO:

• make it work when a core module is loaded that is excluded using opts.builtins (currently it will crash like in Unable to resolve stream module with browserField false #1654)

[dbousque]

Thanks @goto-bus-stop for the fix. I can confirm that it works and fixes an issue I have. Is it planned to release this fix ?

[dbousque]

Thanks @goto-bus-stop for checking again. Is it ready to merge ?

[shuse2]

We are having this problem too.
Would it be possible to merge this PR for the next release?

[ljharb]

this should use resolve’s isCore functionality, i think?

[kevinswiber]

@ljharb I don't believe so, as custom built-ins can be fed into Browserify, and those won't be picked up by resolve.isCore. See: https://github.com/browserify/browserify/blob/master/index.js#L551-L553

[ljharb]

If those can override the default ones, then that's a fair point - but then browserify should be using resolve's node-version-specific logic for the defaults, which it doesn't appear to be.

[kevinswiber]

@ljharb Yes, it overrides the defaults, in a way. It's just a direct replacement of built-in resolution. Browserify doesn't use the node-version-specific logic, and that's a separate issue from this one that's worth logging. I'm maintaining a fork right now with this change, and I'd like to be able to depend on the officially published version. Is there anything else holding this up or can it move forward?

[kevinswiber]

Let me know if there's anything I can do to help push a fix for this over the finish line.

Use case: I'm using Browserify to custom-tailor third-party modules for use within the Postman Sandbox execution environment (see: https://explore.postman.com/templates/7170/browserify-cdn-modules).

Being an Electron app that hosts an even more restricted execution environment, a Browserify CLI call needs to look something like this:

➜ browserify \
--require @stoplight/spectral \
--no-browser-field \
--insert-global-vars __filename,__dirname,process,global \
--exclude assert,buffer,events,fs,os,path,punycode,querystring,stream,string_decoder,timers,url,util

Because of this bug, I get the following error:

Error: ENOENT: no such file or directory, lstat '/Users/kevin.swiber/src/postman-browserify/assert'

This PR fixes the issue for me.

Thanks!