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

Universal normalization of exports / alternative to nested conditional exports #453

Closed
coreyfarrell opened this issue Dec 9, 2019 · 10 comments

Comments

@coreyfarrell
Copy link
Member

Sorry for bringing this up late in the process but #452 (comment) made me think about things and I didn't want to create a tangent in that thread.

I'd like us to consider what it would take to describe an algorithm for normalizing exports so each export could be represented by an array with one condition per array element. This algorithm would require that condition priority is object-order rather than resolver defined so the normalization could be universal (a specific resolver cannot define the priority of unknown keys).

Take the following:

{
  "exports": {
    ".": [{
      "node": [{"never-matches": "./never-matches.js"}, "./throw-not-supported.js"],
      "systemx": "./systemx.js"
    }, "./fallback.js"]
  }
}

The nested structure raises an issue - is systemx a platform or a condition? An alternative which avoids nested conditionals and eliminates ambiguity of systemx:

{
  "exports": {
    ".": [{
      "@node/never-matches": "./never-matches.js",
      "@node": "./throw-not-supported.js",
      "@systemx": "./systemx.js"
    }, "./fallback.js"]
  }
}

In this example conditional keys are @platform, @platform/condition or condition. Characters @ and / are reserved so not allowed as part of platform or condition identifiers. I use @ and / just for demonstration purposes, this can be changed if it would cause confusion over scoped package names.

The normalization of of the exports object could be:

{
  ".": [
    {platform: "node", condition: "never-matches", targets: ["./never-matches.js"]},
    {platform: "node", condition: "default", targets: ["./throw-not-supported"]},
    {platform: "systemx", condition: "default", targets: ["./systemx.js"],
    {platform: "any", condition: "default", targets: ["./fallback.js"]}
  ]
}

In addition to allowing creation of a universal normalization (and possibly resolution search) algorithm this would reduce the potential complexity/depth of the package.json#exports structure. This would be especially nice considering npm will force line-feeds into package.json any time it makes a change, object order would allow users to generally avoid array notation.

This proposal would rule out greedy-matching as nesting would not exist. For blocking of fallback I think this could be accomplished by using a false target:

{
  "@node/require": "./node.cjs",
  "@node": false,
  "default": "./universal.js"
}

This would specify that require() in node would resolve node.cjs, trying to import() on node would fail as the false would prevent node from seeing the default condition.

@jkrems
Copy link
Contributor

jkrems commented Dec 9, 2019

I think the normalization should drop unknown conditions. Because it feels weird if the normalized form has things that are just ignored. Example of what I mean:

{
  "systemx": ["std:so", "std:many", "std:things", "std:to", "std:check"]
}

// Normalize on node to:
[
  // nothing to do
]

Benefit: It makes these a lot more helpful in error messages and debugging. Because node never really tried to check for systemx, I wouldn't include it in a list of things node looked at. It should also show up in a debug log of "things dropped while normalizing".

@jkrems
Copy link
Contributor

jkrems commented Dec 9, 2019

My idea of a normalized pattern would be:

type Conditional = { flag: string, value: any, path: Candidate };
type Candidate = Conditional | string;
type ExportMapping = Candidate[];

That way the normalized form has exactly one representation. E.g. the only array is at the top level. And I don't think the normalized form should know that node is a platform. It should just have a clear list of prioritized conditions. Object order in package.json is super dangerous if any automation touches it. For example all kinds of tools that write back sorted versions.

@coreyfarrell
Copy link
Member Author

@jkrems My complaint with your suggested normalized pattern is type Candidate Conditional | string. I would expect that a normalized structure would not contain any or types. I'm also unclear how your proposed normalization structure would support greedy matching.

Personally I still do not like the nested structure. My issues with nested are potential confusion about greedy vs non-greedy and the result of npm prettifying package.json (npm would forcefully inject line-feeds to the nested exports structures in my above examples). If the decision to support/promote nested structures is a done deal I don't want to be an annoyance so I will try avoiding further comment on it outside this issue.

Take the following example:

{
  "node": {
    "require": "./index.cjs"
  },
  "default": "./index.mjs"
}

Without reading documentation I would intuitively expect that this says ./index.cjs is for node.js require() only, otherwise the universal ./index.mjs is to be used. If I actually wanted to block node.js from resolving the module with import() I would be looking to do something resembling:

{
  "node": {
    "require": "./index.cjs",
    "default": null
  },
  "default": "./index.mjs"
}

Supporting null targets for conditions would be in line with the response on jkrems/proposal-pkg-exports#37.

@jkrems
Copy link
Contributor

jkrems commented Dec 10, 2019

If you don't like the or/nested, an alternate normalized form would be this:

type Conditional = { conditions: string[], path: Candidate };
type ExportMapping = Conditional[];

Taking your example and non-greedy semantics:

{
  "node": {
    "require": "./index.cjs"
  },
  "default": "./index.mjs"
}

// Normalized to:
[
  { conditions: ['node', 'require'], path: './index.cjs' },
  { conditions: [], path: './index.mjs' },
]

Without reading documentation I would intuitively expect that this says ./index.cjs is for node.js require() only, otherwise the universal ./index.mjs is to be used.

This feels orthogonal to the normalized form. It sounds like you're saying "I think non-greedy is more intuitive" which I can definitely understand.

For completeness-sake, we could also express greedy matching in a normalized form by doing something like this:

[
  { conditions: ['node', 'require'], path: './index.cjs' },
  { conditions: ['node', 'require'], path: null }, // abort search here
  { conditions: [], path: './index.mjs' },
]

But it feels a bit confusing because it's really hard to distinguish between "none of the files is applicable for this condition" and "this condition wants to explicitly block any kind of import if it applies".

@coreyfarrell
Copy link
Member Author

This feels orthogonal to the normalized form. It sounds like you're saying "I think non-greedy is more intuitive" which I can definitely understand.

100% for me.

For completeness-sake, we could also express greedy matching in a normalized form by doing something like this:

[
  { conditions: ['node', 'require'], path: './index.cjs' },
  { conditions: ['node', 'require'], path: null }, // abort search here
  { conditions: [], path: './index.mjs' },
]

But it feels a bit confusing because it's really hard to distinguish between "none of the files is applicable for this condition" and "this condition wants to explicitly block any kind of import if it applies".

Did you mean to have the abort entry say { conditions: ['node'], path: null }? Otherwise I think the node.js import() resolver would not match either instance of conditions: ['node', 'require'] so it would still resolve ./index.mjs.

@GeoffreyBooth
Copy link
Member

If the decision to support/promote nested structures is a done deal

It’s certainly not, and that’s open for discussion. (Not that I’m saying that I oppose it, but its complexity makes me concerned.)

As I understand it, and @guybedford or @jkrems correct me please, we have/need nesting because of sugars. For example, we could allow only the most verbose syntax:

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

But because most packages have only one export, Guy wanted to mimic the non-conditional "exports": "./index.mjs" shorthand for conditionals:

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

And because Jan wanted to have explicit prioritization, we also have the array form.

@jkrems
Copy link
Contributor

jkrems commented Dec 10, 2019

Did you mean to have the abort entry say { conditions: ['node'], path: null }?

I simplified but I guess too much. The complete version would be something like this - at the end of each branch is an abort ("at the closing brace"):

[
  { conditions: ['node', 'require'], path: './index.cjs' },
  { conditions: ['node', 'require'], path: null }, // abort search here
  { conditions: ['node'], path: null }, // abort search here
  { conditions: [], path: './index.mjs' },
  { conditions: [], path: null }, // abort search here
]

@jkrems
Copy link
Contributor

jkrems commented Dec 10, 2019

As I understand it, and @guybedford or @jkrems correct me please, we have/need nesting because of sugars.

I don't think I agree. :) We have nesting because a single condition isn't enough to express all use cases we found. Examples:

  1. Have separate require builds for browser and node.
  2. Have separate development/production versions for node and "generically".
  3. Have a development CJS and development MJS.
  4. Have a web2019 build that's specific to the browser and an ES5 fallback for browsers.
  5. [...]

The design goal was to explicitly not just solve for the basic "require vs. import and only works in node" case.

@coreyfarrell
Copy link
Member Author

@guybedford Just to clarify I'm not questioning "exports": {".": {"default": "./index.js"}} vs "exports": {"default": "./index.js"}. The ability to write exports in both of these ways make total sense to me. When I talk about nesting I'm referring specifically to the conditions such as {"node": {"require": "./index.cjs"}}. It's the condition in a condition nesting which troubles me.

@coreyfarrell
Copy link
Member Author

Considering exports now use object order I think it's possible to create a normalization function that does not depend on knowledge of any specific loader.

Also IMO the time for changes to conditional exports has passed so I'm closing this.

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

3 participants