vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.95k stars 26.98k forks source link

Can't resolve dependency when it has only the `browser` field #32714

Closed macabeus closed 2 years ago

macabeus commented 2 years ago

What version of Next.js are you using?

12.0.7

What version of Node.js are you using?

v12.18.4

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

The error happens locally

Describe the Bug

If a dependency doesn't have the field main in the package.json, the error Module not found: Can't resolve 'dependency-name' is raised.

Expected Behavior

It should work since a dependency could not have the field main if it's only for the browser. In this case, the field browser is filled.

NPM doc is reasonable when explaining that it's an "instead of". Otherwise, I'm losing the hint that it's only for the browser.

browser If your module is meant to be used client-side the browser field should be used instead of the main field. This is helpful to hint users that it might rely on primitives that aren't available in Node.js modules. (e.g. window)

To Reproduce

I created an example repository: https://github.com/macabeus/next-js-react-gba-js

> git clone git@github.com:macabeus/next-js-react-gba-js.git
> cd next-js-react-gba-js
> npm run dev

Everything works well.

But if you downgrade the package react-gbajs to 1.0.0, or remove the field main at node_modules/react-gbajs/package.json, the error is raised.

balazsorban44 commented 2 years ago

When using Next.js 12, the minimum required Node version is 12.22.0: https://nextjs.org/docs/upgrading#minimum-nodejs-version

I can also see that you are the maintainer of the package, so I suggest reading this discussion to make sure you bundle/publish the package according to the standards: https://github.com/vercel/next.js/issues/31518#issuecomment-991950855

browser is not a real standard.

macabeus commented 2 years ago

browser is not a real standard.

So the NPM documentation is wrong? It's pretty clear that if the package is only for the browser, it should use browser instead of main. If NPM documentation is wrong, where can I read more about the real standard for package.json?

I can also see that you are the maintainer of the package, so I suggest reading this discussion to make sure you bundle/publish the package according to the standards: #31518 (comment)

I read the discussion starting from the comment that you linked, and it looks like that Pauan is correct when complaining that Nextjs is breaking the standard. I updated my package because it's my toy project, but I don't know if other libraries author are going to do the same.


Furthermore, I had a similar discussion on eslint-plugin-import (https://github.com/import-js/eslint-plugin-import/issues/2132), and in the end, it was marked as an issue in eslint-plugin-import, not in my project.

balazsorban44 commented 2 years ago

So to be more precise.

We don't support when only browser is defined, as we don't run packages only in the browser. Defining a main is a good idea.

So the NPM documentation is wrong?

browser is just a convention that became popular, we do support it but only in the browser compilation, not in the Node.js compilation (logically)

balazsorban44 commented 2 years ago

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.