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

[[ Question ]] Mapping in browser field doesn't affect dependencies #1324

Closed
davegomez opened this issue May 27, 2021 · 11 comments
Closed

[[ Question ]] Mapping in browser field doesn't affect dependencies #1324

davegomez opened this issue May 27, 2021 · 11 comments

Comments

@davegomez
Copy link

I'm trying to replace Browserify in a project I'm working on.

The project bundles many dependencies originally written for Node.js, so they are full of usages of Node's APIs.

Setting the field browser in my project's package.json file maps the modules correctly, but it doesn't work for the dependencies that I also need to bundle.

  "browser": {
    "crypto": "crypto-browserify",
    "fs": "browserify-fs",
    "https": "https-browserify",
    "os": "os-browserify",
    "path": "path-browserify"
  },

Is there a way to do this with esbuild, or am I stuck with Browserify?

@evanw
Copy link
Owner

evanw commented May 28, 2021

Can you provide a sample project that demonstrates the problem?

@davegomez
Copy link
Author

davegomez commented May 28, 2021

Hello @evanw and thank you for answering this.

This is the project I'm trying to migrate to esbuild. In the WIP commit you can see my efforts to move it to esbuild creating the config files in the esbuild directory.

You can visualize the issue I'm talking about by running either npm run build:addon, npm run build:platform-ssb, npm run dev:addon, or npm run dev:platform-ssb.

You will see the errors in the command output and how they ask to use platform: node to handle the node modules in the project dependencies.

@myme
Copy link

myme commented Jun 6, 2021

I'm getting the same issue trying to use:

"browser": {
    "path": "path-browserify"
}

in my package.json. The error is:

Could not resolve "path" (use "platform: 'node'" when building for node)

@evanw
Copy link
Owner

evanw commented Jun 8, 2021

I can't get this to work with any of the bundlers (Browserify, Parcel, Rollup, or Webpack). Here's what I tried:

  • Here's my setup code:

    $ echo '{ "browser": { "path": "path-alternative" } }' > package.json
    $ npm i browserify parcel webpack webpack-cli rollup @rollup/plugin-commonjs @rollup/plugin-node-resolve
    $ mkdir node_modules/pkg node_modules/path-alternative
    $ echo 'console.log(require("pkg"))' > entry.js
    $ echo 'module.exports = require("path")' > node_modules/pkg/main.js
    $ echo '{ "main": "main.js" }' > node_modules/pkg/package.json
    $ echo "module.exports = 'alternative'" > node_modules/path-alternative/index.js

    This installs all of the bundlers and creates a package called pkg that requires path. If the top-level browser substitution works, the package path should be substituted with the package path-alternative which has a single export: the string alternative.

  • Here's what happens when I try Browserify:

    $ ./node_modules/.bin/browserify entry.js | node
    <ref *1> {
      resolve: [Function: resolve],
      normalize: [Function: normalize],
      isAbsolute: [Function: isAbsolute],
      join: [Function: join],
      relative: [Function: relative],
      _makeLong: [Function: _makeLong],
      dirname: [Function: dirname],
      basename: [Function: basename],
      extname: [Function: extname],
      format: [Function: format],
      parse: [Function: parse],
      sep: '/',
      delimiter: ':',
      win32: null,
      posix: [Circular *1]
    }

    Browserify appears to ignore a top-level browser value. If it respected it, this should print the string alternative.

  • Here's what happens when I try Webpack:

    $ ./node_modules/.bin/webpack build ./entry.js --mode=production
    assets by status 279 bytes [cached] 1 asset
    ./entry.js 28 bytes [built] [code generated]
    ./node_modules/pkg/main.js 33 bytes [built] [code generated]
    
    ERROR in ./node_modules/pkg/main.js 1:0-32
    Module not found: Error: Can't resolve 'path' in './node_modules/pkg'
    
    BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
    This is no longer the case. Verify if you need this module and configure a polyfill for it.
    
    If you want to include a polyfill, you need to:
      - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
      - install 'path-browserify'
    If you don't want to include a polyfill, you can use an empty module like this:
      resolve.fallback: { "path": false }

    The build fails completely.

  • Here's what happens when I try Parcel:

    $ ./node_modules/.bin/parcel build entry.js
    ✨ Built in 1.13s
    
    dist/entry.js                                5.69 KB    299ms
    ├── node_modules/path-browserify/index.js       4 KB    120ms
    ├── node_modules/process/browser.js          1.62 KB     30ms
    ├── Code from unknown sourcefiles               52 B      0ms
    └── entry.js                                    19 B    399ms
    
    $ node dist/entry.js
    <ref *1> {
      resolve: [Function: resolve],
      normalize: [Function: normalize],
      isAbsolute: [Function: isAbsolute],
      join: [Function: join],
      relative: [Function: relative],
      _makeLong: [Function: _makeLong],
      dirname: [Function: dirname],
      basename: [Function: basename],
      extname: [Function: extname],
      format: [Function: format],
      parse: [Function: parse],
      sep: '/',
      delimiter: ':',
      win32: null,
      posix: [Circular *1]
    }

    Parcel also ignores a top-level browser value. If it respected it, this should print the string alternative.

  • Here's what happens when I try Rollup:

    $ ./node_modules/.bin/rollup entry.js -p @rollup/plugin-commonjs -p @rollup/plugin-node-resolve
    
    entry.js → stdout...
    import require$$0 from 'path';
    
    var entry = {};
    
    var main = require$$0;
    
    console.log(main);
    
    export default entry;

    Rollup doesn't appear to notice the top-level browser value either. The import path is still path.

It's possible that I have set this up incorrectly. If so, please let me know how to adjust this test case such that it works. But under the assumption that there is no bundler that supports this, esbuild shouldn't support this either.

If you still want to use esbuild, you can substitute path with a plugin instead: https://esbuild.github.io/plugins/#resolve-callbacks.

@davegomez
Copy link
Author

Hello @evanw, thank you for taking the time to look into this.

I can't tell you if your setup is correct to mimic my use case, I'm not that good. The only thing I can tell you is the project I'm trying to port to esbuild is already using Browserify to bundle the same files I'm trying to bundle as you can see in the master branch.

And these are the only Browserify configuration options that is using:

  "browserify": {
    "transform": [
      [
        "sveltify",
        {
          "extensions": [
            ".svelte"
          ],
          "svelte": {
            "dev": false
          }
        }
      ],
      [
        "scssify"
      ]
    ]
  }

The project is bundling the files directly with scripts like this:

"build:addon": "browserify src/main.js -o dist/bundle.js",

I really don't understand what is going on.

@evanw
Copy link
Owner

evanw commented Jun 8, 2021

I think what's happening is that Browserify has built-in behavior to substitute path with path-browserify but other bundlers such as esbuild and Webpack 5 do not. The browser mapping you are trying to specify is being ignored by all bundlers and what you are observing instead is the bundler's default behavior.

One thing you could try to debug further is specifying another mapping other than the one built into Browserify. For example, instead of "path": "path-browserify" you could try "path": "left-pad" or something and then see what the result of require("path") is after Browserify runs. If it's still the path-browserify package instead of the left-pad package then you know your browser mapping is being ignored and has no effect.

I can't tell you if your setup is correct to mimic my use case, I'm not that good.

I appreciate you providing a repository that reproduces the issue. However, it has a lot of dependencies and I wasn't able to install some of them. So I made a minimal test case to try to get to the root of the issue.

@davegomez
Copy link
Author

I'm back @evanw. Sorry for the delay but it has been a very difficult last month for me.

I created a small repo to illustrate what is going on with Browserify and esbuild when there are deep dependencies using Node's APIs.

I hope it helps to figure out what Browserify is doing.

IMO this is essential for esbuild if we want the tool to bundle applications for browser-only usage (UMD files).

Repo: patchfox-esbuild

@davegomez
Copy link
Author

Bump

@ghost
Copy link

ghost commented Aug 30, 2021

@evanw

If you still want to use esbuild, you can substitute path with a plugin instead: https://esbuild.github.io/plugins/#resolve-callbacks.

This was very helpful and insightful, thank you for the very descriptive demo. I went ahead and extended on your suggestion of using resolve callbacks. Could you take a look at this minimal repo to see if that's in line with what you had in mind?
https://github.com/mi544/esbuild-module-substitution

@davegomez
I've run into a similar problem to the one you mentioned in this issue and this may be a solution you're looking for. Could you take a look at the repo as well to see if it's doing what you need it to?

Thank you

@ghost
Copy link

ghost commented Aug 30, 2021

I will also say that while bundling the dependencies with ESBuild that way worked just fine, along with replacing certain dependencies of my dependencies, the dependency that required all that manipulation wasn't very functional with browserify modules in the end. That probably goes back to the fact that libraries written for Node shouldn't be used in the browser in the first place. That's not to say you can't do it - you get full control over how bundling is carried out in your project and that is always a good thing.

@evanw
Copy link
Owner

evanw commented Apr 6, 2022

I'm going to close this as I believe esbuild is working as intended here. The browser field appears to have been designed for package authors to enable their package to work in the browser. I don't believe it's designed for package consumers to get packages to work in the browser. The authors of these node-only packages could update their package to be browser-friendly by providing browser mappings for their packages, which esbuild would then respect. Or, if you want to override these paths bundle-wide with esbuild, you can use an esbuild plugin to do this.

@evanw evanw closed this as completed Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants