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

export spec clarification with require xor import and non-JS cases #41686

Closed
evanw opened this issue Jan 25, 2022 · 22 comments
Closed

export spec clarification with require xor import and non-JS cases #41686

evanw opened this issue Jan 25, 2022 · 22 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@evanw
Copy link

evanw commented Jan 25, 2022

Affected URL(s)

https://nodejs.org/api/packages.html#conditional-exports

Description of the problem

I'm coming here from evanw/esbuild#1956. Specifically the esbuild bundler implements node's spec for exports in package.json files. Node's documentation for exports says this:

  • "import" - matches when the package is loaded via import or import(), or via any top-level import or resolve operation by the ECMAScript module loader. Applies regardless of the module format of the target file. Always mutually exclusive with "require".
  • "require" - matches when the package is loaded via require(). The referenced file should be loadable with require() although the condition matches regardless of the module format of the target file. Expected formats include CommonJS, JSON, and native addons but not ES modules as require() doesn't support them. Always mutually exclusive with "import".
  • "default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.

This is what esbuild implements. The case I'm looking for clarification on is that there are packages that just have require and import but not default. This is reasonable as the example in the documentation also doesn't include a default condition, and the documentation says import and require are mutually exclusive (i.e. exactly one should be present).

However, esbuild performs the following kinds of path resolution:

  • import-statement
  • require-call
  • entry-point
  • import-rule
  • url-token

I understand what should happen with the first two but not with the last three. An entry point is path resolution that happens due to a package name provided on the command line. There is no import or require in that case. And the last two happen when you import things from CSS (something a bundler has to deal with) where there is also no JS import statement or require call. If esbuild supported importing things from HTML, which it may one day, there would also be those cases to consider.

Right now esbuild handles these non-JS cases by still running the exports logic but just without the import or require conditions active, since it's neither an import nor a require. But then that breaks when there's no default condition. Do you have any opinion for what should happen here, as the authors of this specification? Some options:

  • Don't apply import or require and fail path resolution if nothing applies (esbuild's current behavior)
  • Don't apply import or require and fall back to legacy module/main fields if nothing applies
  • Pick one of import or require to apply arbitrarily
  • Apply both import and require
  • Don't attempt to apply exports rules at all and only use legacy module/main fields
  • Undefined: up to the tool to do whatever they want
  • Something else?
@evanw evanw added the doc Issues and PRs related to the documentations. label Jan 25, 2022
@Trott
Copy link
Member

Trott commented Jan 25, 2022

@nodejs/modules

@ljharb
Copy link
Member

ljharb commented Jan 25, 2022

A package name on the command line is “whatever the package’s package.json says the format of the entry point is”, which is deterministic from the file extension of the main/exports dot, and optional “type” field.

There wouldn’t be any standard behavior for module types that don’t exist, like css or html.

@evanw
Copy link
Author

evanw commented Jan 25, 2022

Ok, that makes sense. I'll do that. Thanks!

@jkrems
Copy link
Contributor

jkrems commented Jan 25, 2022

If esbuild supported importing things from HTML, which it may one day, there would also be those cases to consider.

Technically from HTML there's a valid 3rd option ("script"?) in addition to require and import. But I'm not sure how valuable supporting that would be in a relatively new bundler. For <script type="module">, it seems fairly unambiguously import.

Right now esbuild handles these non-JS cases by still running the exports logic but just without the import or require conditions active, since it's neither an import nor a require.

That feels sensible, partially also because I'm not sure what the semantics are of "including JS from CSS" (does it fail if the CSS isn't imported in turn, transitively, by some other JS..?). Effectively, this seems like something that might have to be up to tools because these non-JS edges may have wildly different semantics across tools (e.g. import-CSS as a side effect vs. as importing a stylesheet object). My guess is that just considering those to be import edges is the most useful choice because that appears to be the authoring trend for reusable front-end code.

But my cop-out answer would be "node isn't in the best position to make a recommendation because it lacks domain expertise". I would expect this question to require a chat between parcel/esbuild/webpack/rollup... because in that group there's a higher chance of an informed outcome. :)

@guybedford
Copy link
Contributor

Node.js itself does not support package resolution for the entry point so does not hit this problem. Node.js mains can only be direct file system paths, and then Node.js uses the standard format algorithms (as mentioned by @ljharb) to determine the initial module format.

So the entry-as-a-package problem is unique to build tools. In my own tooling I make "import" the first condition by default so that ES modules are preferred to CommonJS. If that leads to the dual module issues, then perhaps having a specific configuration to override this default would be useful.

@evanw
Copy link
Author

evanw commented Jan 25, 2022

Thanks so much for your replies.

I'm not sure what the semantics are of "including JS from CSS"

I'm not sure either. This just isn't allowed in esbuild (it's a build error). I think this case would be more for other non-JS assets that end up using the exports map too. People publish all kinds of things to npm. JS assets do come up in the entry point and HTML cases though.

A package name on the command line is “whatever the package’s package.json says the format of the entry point is”, which is deterministic from the file extension of the main/exports dot, and optional “type” field.

I don't think the file extension applies here because the command line isn't a file, and doesn't have an extension. This corresponds to someone doing esbuild --bundle pkg-name. And you can't use the file extension of the default file for the package because you don't have that information yet; you need to figure out what condition to apply to learn what the default file for the package is.

What I took this to mean was to inspect the "type" field in the package.json file and use the require condition unless "type": "module" is present, in which case the import condition is used instead. That makes sense to me because this is an existing behavior and then the behavior in this edge case is determined by the package author's decisions, so it's sort of under their control what happens.

In my own tooling I make "import" the first condition by default so that ES modules are preferred to CommonJS. If that leads to the dual module issues, then perhaps having a specific configuration to override this default would be useful.

I could also see "import should be the default" or "it's up to the build tool" for the entry point case because there are arguments for either. The person who filed the original issue with me wanted the selection to be determined by the bundler's configured output format, which is also reasonable. I mainly wanted to check here first to see if there was a definite answer to what I should do (especially if this has come up before) so I don't end up violating the specification. But it's ok if the answer is that it's left unspecified too.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2022

@evanw type module changes what .js means but it doesn’t force import; i can still require a package with type module if it’s main/dot points to a .cjs file (and probably also a .json file)

@evanw
Copy link
Author

evanw commented Jan 25, 2022

i can still require a package with type module if it’s main/dot points to a .cjs file

I believe you're talking about the extension of "." inside of "exports". However, what we are trying to figure out is whether to apply the import or require condition so we can evaluate "." inside of "exports" in the first place. So what you're suggesting is circular because you'd need the information to compute itself, so that information isn't available (if I understand you correctly). For example, consider esbuild --bundle pkg-foo where the package pkg-foo has a package.json file that looks like this:

{
  "name": "pkg-foo",
  "exports": {
    ".": {
      "import": "./foo.mjs",
      "require": "./foo.cjs"
    }
  }
}

We are trying to figure out what conditions to apply so we can figure out what file to use as the entry point. We don't know whether to apply import or require at this point because the provided package path pkg-foo came from the command line and not from an import statement or a require call. We can't use the extension of the entry point file because there isn't just one, and there's no default condition so there's no canonical one.

Maybe you're saying esbuild should just not apply the "exports" conditions at all in this case and instead fall back to the legacy main field lookup instead?

@bmeck
Copy link
Member

bmeck commented Jan 25, 2022

this seems to have some overlap with determining which loader to initiate for the entry point as in #41552

@ljharb
Copy link
Member

ljharb commented Jan 25, 2022

@evanw i see what you mean. since node's default entry point type is CJS, and will remain so for the foreseeable future, that's what i'd suggest assuming.

Packages that aren't concerned with backwards compatibility (imo, user-hostile ones) omit "main" entirely, so a fallback to "main" isn't something you can rely on.

@bmeck
Copy link
Member

bmeck commented Jan 25, 2022

@ljharb the entry point isn't CJS there is a function that determines which loader is used

@ljharb
Copy link
Member

ljharb commented Jan 25, 2022

@bmeck Good to know! If it's a package name then, how does that function determine it?

@bmeck
Copy link
Member

bmeck commented Jan 25, 2022

@ljharb CLI args are always converted to a file path (either as url or path string depending) so you cannot ref a package name.

@ljharb
Copy link
Member

ljharb commented Jan 25, 2022

gotcha. and what if i do node node_modules/foo?

@bmeck
Copy link
Member

bmeck commented Jan 25, 2022

@ljharb that is actually complex... it first does a CJS resolve against the full file path (normally , not always) and then determines if it should do an ESM or CJS load operation which can be a bit thrashy

@ljharb
Copy link
Member

ljharb commented Jan 25, 2022

Sounds like "CJS is the default" would apply, then, altho with an ESM fallback.

@bmeck
Copy link
Member

bmeck commented Jan 25, 2022

@ljharb that isn't what happens though

@bmeck
Copy link
Member

bmeck commented Jan 25, 2022

function shouldUseESMLoader(mainPath) {
const userLoader = getOptionValue('--experimental-loader');
if (userLoader)
return true;
const esModuleSpecifierResolution =
getOptionValue('--experimental-specifier-resolution');
if (esModuleSpecifierResolution === 'node')
return true;
// Determine the module format of the main
if (mainPath && StringPrototypeEndsWith(mainPath, '.mjs'))
return true;
if (!mainPath || StringPrototypeEndsWith(mainPath, '.cjs'))
return false;
const pkg = readPackageScope(mainPath);
return pkg && pkg.data.type === 'module';
}

@evanw
Copy link
Author

evanw commented Jan 25, 2022

I just ran a test of the other bundlers: https://github.com/evanw/entry-point-resolve-test. Webpack and Rollup are consistent with each other, and Parcel just consistently fails to handle this case. It seems like the rules should be as follows:

  1. If exports exists, try resolving with import, and fail if nothing matches
  2. Otherwise, try module
  3. Otherwise, try main

Since the other bundlers are consistent and it sounds like perhaps there isn't strong consensus here, I think I should probably just do what they do.

@GeoffreyBooth
Copy link
Member

  1. If exports exists, try resolving with import, and fail if nothing matches

I agree with this approach. I think for most cases, if the author has included "exports" and "import" it’s a pretty modern package and they probably expect the ESM version to be used first with the CommonJS version as the fallback. I don’t have numbers but I’d bet that ESM with CommonJS fallback is a far more common pattern than CommonJS with ESM fallback.

Where does "exports" "require" fit into the algorithm? Between steps 2 and 3? As part of step 1, after trying import and default?

@ljharb
Copy link
Member

ljharb commented Jan 26, 2022

Couldn’t the ordering of the conditions matter? That way, whatever they put first is the priority.

@evanw
Copy link
Author

evanw commented Feb 9, 2022

I went with import semantics for this in esbuild by the way, as described above. So I don't need this issue open anymore. Feel free to close it if you don't need it either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

7 participants