esbuild icon indicating copy to clipboard operation
esbuild copied to clipboard

Incorrect warning regarding "default" conditional export

Open tvsbrent opened this issue 1 year ago • 3 comments

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?

tvsbrent avatar Aug 20 '24 13:08 tvsbrent

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.

hyrious avatar Aug 20 '24 14:08 hyrious

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.

tvsbrent avatar Aug 20 '24 15:08 tvsbrent

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.

evanw avatar Aug 21 '24 01:08 evanw

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)

bhousel avatar Sep 09 '24 19:09 bhousel

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

evanw avatar Sep 10 '24 15:09 evanw