Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Conditional exports as flags #428

Closed
sokra opened this issue Nov 13, 2019 · 15 comments
Closed

Conditional exports as flags #428

sokra opened this issue Nov 13, 2019 · 15 comments

Comments

@sokra
Copy link

sokra commented Nov 13, 2019

Is your feature request related to a problem? Please describe.
Currently node supports three "conditions" for resolving conditional exports:

  • require: For using require() instead of import
  • node: For targeting node
  • default: as general fallback

For other tooling other conditions are recommended:

  • browser: For targeting the browser.
  • electron: For targeting electron
  • deno: For targeting deno
  • react-native: For targeting react-native

I would argue that these conditions fall into two orthogonal categories: target and usage.

i. e. one could use require while targeting node, or one could use require while targeting browser.

Currently it's a bit unclear if require means using require() in node.js or using require() in general for any target.

If it means using require() in general for any target there would be no way to target only CJS usage in node.js.

If it means using require() in node.js we probably have to get creative when somebody wanting to target CJS usage in browser, i. e. with a new condition browser-require. But this also means it would become inconsistent with the node conditions and cause user confusion: Why is "require" not used for my frondend package?

Example
"exports": {
  "require": "./node-cjs.js",
  "node": "./node-esm.js",
  "browser-require": "./browser-cjs.js",
  "browser": "./browser-esm.js",
  "default": "./whatever.js"
}

Describe the solution you'd like
Treat conditions as flags and allow combinations of them.

One solution could be to separate combinations with +, which would result in a minimal change I would recommend:

 The conditions supported in Node.js are matched in the following order:

+ 1. `"node+require"` or  `"require+node"` - matched when the package is loaded via `require()` 
+  for any Node.js environment.
+   _This is currently only supported behind the
+   `--experimental-conditional-exports` flag._
  2. `"require"` - matched when the package is loaded via `require()`.
    _This is currently only supported behind the
    `--experimental-conditional-exports` flag._
  3. `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
     module file. _This is currently only supported behind the
    `--experimental-conditional-exports` flag._
  4. `"default"` - the generic fallback that will always match if no other
     more specific condition is matched first. Can be a CommonJS or ES module
     file.

This would allow other tooling for support combinations of require with other targets:

"exports": {
  "node+require": "./node-cjs.js",
  "node": "./node-esm.js",
  "browser+require": "./browser-cjs.js",
  "browser": "./browser-esm.js",
  "default": "./whatever.js"
}

Describe alternatives you've considered
Other separators would also be possible: i. e. node-require or node/require.

It's possible to restrict the allowed order of conditions, i. e. target must come before usage.

It's also possible to use nested objects to specify combinations of conditionals, which isn't that bad, but more verbose:

"exports": {
  "node": {
    "require": "./node-cjs.js",
    "default": "./node-esm.js"
  },
  "browser": {
    "require": "./browser-cjs.js",
    "default": "./browser-esm.js"
  },
  "default": "./whatever.js"
}

Now that I see that, I may like it a little bit more... But it's a bit more work to implement that.

cc @guybedford
cc @lukastaegert for rollup

@sokra
Copy link
Author

sokra commented Nov 13, 2019

Which such a system in place you could consider additional conditions like:

  • wasm or experimental-wasm: wasm support is enabled (--experimental-wasm-modules)

This would make it possible to import "wasm" while still providing a fallback solution when wasm is not enabled.

@MylesBorins MylesBorins transferred this issue from nodejs/node Nov 13, 2019
@MylesBorins
Copy link
Contributor

Transferred to the modules repo for discussion.

The syntax of specifying the target / usage at the top level is sugar that was added last minute... export keys are often defining deep imports, which can make this pattern get super verbose. The following isn't supporting but is something discussed in the last meeting

"exports": {
  ".": {
    "node": {
      "require": "./node-cjs.js",
      "default": "./node-esm.js"
    },
    "browser": {
      "require": "./browser-cjs.js",
      "default": "./browser-esm.js",
      " any-key-you-want": "./wat.wasm"
    }
  },
  "./feature": {
    "default": "./lib/feature-umd.js",
    "node": "./lib/feature.mjs",
    "whatever-you-want": "./lib/feature-in-weird-format.whatever"
  }
}

To be honest I am not totally sold on the current default condition names but have yet to come up with something better... time to think about this a bit more 😇

@jkrems
Copy link
Contributor

jkrems commented Nov 13, 2019

I have to admit that I somewhat lost track of which conditional features we removed. Nested conditionals are actually something that "just worked" initially because it naturally fell out of the recursive definition of export mappings.

The intention was definitely that require matches all uses of CJS, both in node and outside of node. Just like something like production may match a production-optimized version of the code (if NODE_ENV-equivalent conditionals are a thing).

I would argue that these conditions fall into two orthogonal categories: target and usage.

I would call all of them "environment constraints". E.g. require is for an environment that only supports require but cannot use import; node is for an environment that offers node built-in APIs. Even node and browser are orthogonal - there may be environments that do support both API surfaces. IIRC we have a note saying "environments are responsible for prioritizing keys if they support multiple ones".

wasm or experimental-wasm: wasm support is enabled (--experimental-wasm-modules)

Feature detection is super interesting in general. There were some earlier discussion here, including testing for specific APIs: jkrems/proposal-pkg-exports#29. I don't think we've fully determined where we'll ultimately draw the line, especially because there is a risk that a library being too aggressive in pre-processing its code could actually lead to issues when trying to optimize for environments on the app side. So I'm very tempted to prefer something like "source" or the "featureset2019" idea promoted by @mathiasbynens which could cover things like WASM implicitly (not sure if there are good docs on it..?).

@bmeck
Copy link
Member

bmeck commented Nov 13, 2019

require is a runtime set flag, should flags be able to be set by user code at runtime?

@jkrems
Copy link
Contributor

jkrems commented Nov 13, 2019

require is a runtime set flag, should flags be able to be set by user code at runtime?

Do you mean "require is a flag that isn't consistently set across the whole process"? To me node has effectively two "environments" now: The require environment and the import environment. Each is following its own rules. So from that perspective, require is 100% static and cannot change at runtime. Currently node doesn't expose a way to set different conditional orders per environment. I think that's a good thing. And in general our direction seems to be that resolution behavior cannot be changed once user code started running.

@bmeck
Copy link
Member

bmeck commented Nov 13, 2019

@jkrems i ask because this may impact response to https://github.com/littledan/proposal-module-attributes which does allow setting key/value pairs

@jkrems
Copy link
Contributor

jkrems commented Nov 13, 2019

Ah, thanks for clarifying! So, straw person:

// Usage:
import("ui-library", { prefer: ["purple"] });

// ui-library/package.json
{
  "exports": {
    "purple": "./theme-purple.mjs",
    "default": "./theme-default.mjs"
  }
}

My gut feeling is that user code should use specifiers to express what they are looking for. If module attributes ends up as something more generic (e.g. import 'foo' as data), I could imagine that being a conditional though. E.g. an export mapping could have a "static data" variant and an "executes code" variant. But I personally wouldn't want to create a new communication channel for arbitrary signals between usage and export declaration.

@guybedford
Copy link
Contributor

Nested conditions do fall out of both the current spec and implementation, and composition in this was is definitely an important feature. Nesting was the simplest implementation and spec to achieve this given that it was work not to include this feature!

We could well consider a "wasm" environment condition, but note that it would have different meaning to "require", as "require" distinguishes between the CJS and ESM loaders, while "wasm" would ideally be a condition defined to mean that the whole environment supports Web Assembly modules. Having this as a default condition supported in Node.js core might well make sense - perhaps let's open a separate issue to discuss this for --experimental-wasm-modules?

There are also plans to open up setting the environment conditions when running Node.js - something like node --env=wasm,development main.js. The new provided conditions would then be matched in order, all with higher precedence than the Node.js core conditions. If any one wants to PR that please do too.

@sokra
Copy link
Author

sokra commented Nov 14, 2019

Ok seems like we are on the same direction. I think I will start implementing exports and conditional exports for webpack 5 and come back with my experience on that way...

@guybedford
Copy link
Contributor

@sokra that would be really great, and please do provide your feedback here. Implementer feedback from tooling is exactly what we were hoping for before unflagging these conditions for January.

@devongovett
Copy link

I'd also mention the package.json#targets proposal which allows package authors to define arbitrary targets and let tools read this metadata to decide which entry to choose. The existing conditions are mostly to do with the environment the code runs in (browser, node, CJS, ESM), but if, for example, you wanted to have multiple of those for different versions of browsers or node, there's no way to do it. targets allows defining additional metadata (e.g. engines) per entry so tools can make an informed decision about which to choose.

This was originally proposed in the Parcel 2 RFC. Our alpha releases currently support it for building multiple targets, but not yet resolving other packages in node_modules using the same rules. I would like to get buy in from other tools around this.

@MylesBorins MylesBorins added the modules-agenda To be discussed in a meeting label Nov 25, 2019
@SMotaal
Copy link

SMotaal commented Nov 25, 2019

I'd like to us to also reconsider based the direction of these requirements if it makes sense to offer flat/hybrid lookups.

Simple example (@ here used for symbolic effect only):

{
  "exports": {
    "./": "./",
    "./@browser": "./browser/",
    "./helpers": "./helpers.js",
    "./helpers@browser": "./helpers.js"
  }
}

Flat/hybrid lookups make it easier to write the lookup logic as map[`${specifier}@${target}`] || map[specifier]... etc., but also makes it possible for runtimes to opt for and prioritize multiple target fallbacks.

The added cost for platforms with limited capacity can be eliminated during parsing (ie in a JSON.parse(…) function) which is likely cleaner and less taxing for these platforms compared to the alternative of retaining and lazy resolving against multiple lookups.

Just a thought :)

@jkrems
Copy link
Contributor

jkrems commented Nov 25, 2019

One possible downside of encoding additional information in the specifier (especially after ./) is that it removes valid URLs and/or file paths. E.g. foo/@browser is a perfectly fine specifier right now and it would be a breaking change to say that mapping the specifier will stop working.

@SMotaal
Copy link

SMotaal commented Nov 25, 2019

@jkrems certainly, so I opted for @ here only symbolically — I personally thought it premature to want to get into the weeds of it, but : imho would be the safe choice, not to mention ? and # to be ergonomically aligned with those being URL-related restrictions (unless we want to support special cases for query and fragments, not sure if there are use cases out there).

@bmeck
Copy link
Member

bmeck commented Jan 21, 2021

this specific request is available as --conditions, please make a separate issue on flattening if it is still desired.

@bmeck bmeck closed this as completed Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants