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

Incorrect warning regarding "default" conditional export #3887

Closed
tvsbrent opened this issue Aug 20, 2024 · 5 comments
Closed

Incorrect warning regarding "default" conditional export #3887

tvsbrent opened this issue Aug 20, 2024 · 5 comments

Comments

@tvsbrent
Copy link

tvsbrent commented Aug 20, 2024

With the change introduced for this issue, #3867, we are now seeing the following error from esbuild:

▲ [WARNING] The condition "default" here will never be used as it comes after both "import" and "require" [package.json]

The package.json in this case has these conditional exports:

  "exports": {
    ".": {
      "node": "./dist/index.js",
      "require": "./dist/index.js",
      "import": "./dist/index.esm.js",
      "default": "./dist/index.esm.js"
    },
    "./dist/index.css": {
      "default": "./dist/index.css"
    }
  },

According to the NodeJS documentation, it appears default should always be last, so this warning feels incorrect?

@hyrious
Copy link

hyrious commented Aug 20, 2024

The Node.js documentation also said:

Within the "exports" object, key order is significant. During condition matching, earlier entries have higher priority and take precedence over later entries. The general rule is that conditions should be from most specific to least specific in object order.

The matching algorithm should test conditions from top to bottom, until some case matches. Since there's only 2 importing style in JavaScript (require or import), they must at least much one. Thus the default condition will never be used if present after import and require. So the warning is correct here to inform package authors to write correct package.json.

@tvsbrent
Copy link
Author

Based on reviewing the code in that change further and your comment @hyrious , I am going to close this. I'm worried there may be some weird tool in our toolchain that doesn't work as one would hope, and that default is needed in that case, but I don't have a specific situation right now.

@evanw
Copy link
Owner

evanw commented Aug 21, 2024

I'm reopening this as I agree that this warning shouldn't trigger for default like this in my opinion. I missed this case due to an oversight. I'm not sure how it could come up in practice (maybe some tool doesn't support import or require?) but the ability to have a catch-all default clause without a warning seems reasonable to me. Sorry about the warning noise in the meantime. You can hide the warning with --log-override:package.json=silent for now if you'd like.

@bhousel
Copy link

bhousel commented Sep 9, 2024

I'm also seeing this warning for several of my projects that are set up like this:

  "exports": {
    "import": "./thing.mjs",
    "require": "./thing.cjs",
    "*": "./*"
  },

I've used this * export in cases where the project exports both code and data - where consumers might need to do import stuff from 'mypackage/data/stuff.json';

(this might not be best practice though)

@evanw
Copy link
Owner

evanw commented Sep 10, 2024

I'm also seeing this warning for several of my projects that are set up like this:

  "exports": {
    "import": "./thing.mjs",
    "require": "./thing.cjs",
    "*": "./*"
  },

I've used this * export in cases where the project exports both code and data - where consumers might need to do import stuff from 'mypackage/data/stuff.json';

(this might not be best practice though)

Sorry, I'm confused. How would that work? In the example you gave, that import is a module instantiation error. Running that example in node gives the following error:

node:internal/modules/esm/resolve:303
  return new ERR_PACKAGE_PATH_NOT_EXPORTED(
         ^

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './data/stuff.json' is not defined by "exports" in node_modules/mypackage/package.json imported from entry.mjs
    at exportsNotFound (node:internal/modules/esm/resolve:303:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:650:9)
    at packageResolve (node:internal/modules/esm/resolve:828:14)
    at moduleResolve (node:internal/modules/esm/resolve:918:18)
    at defaultResolve (node:internal/modules/esm/resolve:1148:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:528:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:497:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:231:38)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:97:39)
    at link (node:internal/modules/esm/module_job:96:36) {
  code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'
}

Node.js v22.0.0

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