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

Bug: Wrong use of the package.json browser field #58

Open
Download opened this issue Jan 20, 2020 · 6 comments
Open

Bug: Wrong use of the package.json browser field #58

Download opened this issue Jan 20, 2020 · 6 comments

Comments

@Download
Copy link

I notice in this example that it is referencing the umd build in the dist folder in the browser field of package.json:

{
  "main": "dist/how-long-till-lunch.cjs.js",
  "module": "dist/how-long-till-lunch.esm.js",
  "browser": "dist/how-long-till-lunch.umd.js",
}

Thinking of interop with other tools, I wonder whether this is right?

I think Browserify and Webpack both use the browser field as having the meaning 'the entry point for this package, for browsers'. But they assume 'normal' cjs code.

For example have a look at the browser field in the package.json of debug:

{
  "main": "./src/index.js",
  "browser": "./src/browser.js",
  "unpkg": "./dist/debug.js"
}

Notice how they use a file in the src folder here. Both Browserify and Webpack will pick up this file and package / bundle it, wrapping it with a umd wrapper for umd builds.

So I wonder, is it right that the file this project is setting for the browser field is a umd build, that already has a umd wrapper? Won't this file end up being wrapped twice?

@Ivaylo-Lafchiev
Copy link

Debug's setup is actually quite interesting, they don't create and publish a /dist directory, but instead just write their source code in the specific format in /src, removing the need to use rollup in the first place.

Most transpilers in webpack (i.e. babel-loader) recommend excluding node_modules from transpilation, as it is assumed that stuff in node_modules is already transpiled, as is the case with debug.

Even if that wasn't the case, something like babel should be smart enough not to wrap a file that doesn't need wrapping.

@Download
Copy link
Author

Found this specification for the browser field, which suggests it is indeed wrong.

@Download
Copy link
Author

Download commented Jan 22, 2020

According to the spec above, the browser field is either:

  • A string with the path to the alternative file to use for main (so ES5), or
  • An object containing a mapping of file names and their alternatives for use in the browser (in which case the alternative for module must be a module itself).

The browser field is not about transpiling or packaging. It is about allowing library writers to specify which alternative modules should replace which normal modules when bundling for the browser. Because a module is using file I/O in node and needs an alternative (using e.g. localStorage) for the browser, for example.

As far as I can tell, there is no tool that expects those modules to be umd builds and they will wrap them a second time. The more I read about this, the more I am getting convinced this example project is doing it wrong.

@eventualbuddha
Copy link
Contributor

Yep, sounds like it’s wrong. Who wants to fix it?

@Download Download changed the title Is the use of the package.json browser field in this example correct? Bug: Wrong use of the package.json browser field Jan 23, 2020
@justin-calleja
Copy link

@eventualbuddha and by fix you mean delete the browser entry in package.json?

@Download
Copy link
Author

Download commented May 5, 2021

Yes, probably. And the part in the README about it.
Then of course the question remains, how do we deal with differences between Node and Browsers in a library. So apart from deleting the wrong info on the browser field, we should tell the story about how to use that field to use Node specific features on Node and browser features on the browser. Webpack and Browserify solve this with the browser field. As far as I can tell, Webpack actually does use the browser field, even for node modules. It either includes the file specified in main if building for Node or the one(s) specified in the browser field if building for the browser.

Now here is the weird part. As far as I understand it (haven't been using Rollup in a while), Rollup fully supports the browser field. But somehow the advice here is wrong? This actually conflicts with how Rollup works?

But I'm betting that if you would put this example project to the next level and make it more complete you would discover both how the current advice won't work if you follow it and how you should use the browser field to actually do make it work. I think you should give this library a code branch for Node / browsers. Say, read something from env vars on Node or localStorage in the browser. Just a few lines of code. Then use the browser field to specify that on browsers you want to include /src/read.browser.js i.s.o /src/read.js. Publish that and use it in an application project with Rollup and test it. That's the way how to figure it out.

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

4 participants