feat: sort types and default in exports field

[fisker]

Fixes #294
Fixes #239

In this PR:

1. Move subpathes above conditions (I don't think anyone would mix subpathes and conditions, but I don't want types above subpathes if exists)
2. Move 'types' to be the first condition
3. Move 'default' to be the last condition
4. Move 'module-sync' to above 'require'

I don't mean to close #336, but I think we should discuss about other condition orders first before we take further actions. This PR just picking the low-hanging fruits first.

[GeorgeTaveras1231]

@fisker let me know where you would like to have the conversation about the other conditions.

I will call out 1 issue with this implementation:

If you have

{
  "require": "./require.js",
  "import": "./module.mjs",
  "module-sync": "./sync-module.mjs"
}

This will be sorted as

{
  "module-sync": "./sync-module.mjs",
  "require": "./require.js",
  "import": "./module.mjs"
}

This is wrong because import and module-sync are not equal. They have different expectations of the module - import allows top-level await while module-sync does not. But when importing the module using ESM syntax, the resolver will use module-sync if it comes before import.

This means that if a package developer intentionally wants to use a module with top-level await in the import export, they no longer will be making that code available. So this breaks the way the package is read.

[fisker]

let me know where you would like to have the conversation about the other conditions.

Raise an issue.

[keithamus]

Will merging this cause new issues? Or is this a useful stepping stone? Is the status quo broken currently?

[fisker]

I think either moving require down or moving module-sync up can break edge cases.

Let's leave module-sync alone.

[fisker]

@GeorgeTaveras1231 Are you fine with this change now?

[GeorgeTaveras1231]

@fisker Yea this is okay

I've been looking more into the typescript resolves files and am having doubts about types being first 😅. From what I can tell, they don't say anything about this in their docs. Also, after doing some tests I found that types works as a fallback (like default) if tsc does not find a .ts file for the other conditions.

So having the following is valid

/// tsconfig.json
{
  "compilerOptions": {
    // Configure node resolution
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
  }
}

/// package.json["exports"]
{
  "node": "./node-only.js",
  "types": "./fallback-types.d.ts"
}

Where you may have a file ./node-only.d.ts that exposes node-specific types.

Separately, tsc supports the types@{selector} condition - which have to be sorted by the matching range (more specific at the top).

// package.json["exports"]
{
    "types@>=5.2": "./ts5.2/subpath/index.d.ts",
    "types@>=4.6": "./ts4.6/subpath/index.d.ts",
    "types": "./tsold/subpath/index.d.ts",
    "default": "./dist/subpath/index.js"
}

Using the code in this PR will break this example ☝🏽

I don't want to prevent your sense of progress so it's up to you how you want to frame this.

I will open an issue proposing a solution for the other known conditions and will give some thought to the issues with the types condition.

[fisker]

The "types" condition should always come first in "exports"

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#:~:text=The%20%22types%22%20condition%20should%20always%20come%20first%20in%20%22exports%22.

This condition should always be included first

https://nodejs.org/api/packages.html#:~:text=This%20condition%20should%20always%20be%20included%20first.

[fisker]

There is nothing to doubt, they made it very clear since the begining, if it doesn't work. Should be their problem.

[GeorgeTaveras1231]

@fisker good finds. It's certainly odd the way it works and how that relates to the recommendation to place it first.

It takes more effort to properly add type variants this way.

Instead of

{
  "node": "./node-only.js",
  "types": "./fallback.d.ts"
}

will require writing

{
  "types": {
    "node": "./node-only.d.ts",
    "default": "./fallback.d.ts"
  },
  "node": "./node-only.js"
}

But oh well 🤷🏽

[keithamus]

@GeorgeTaveras1231 are you supportive of this change (regardless of the conversation in #336)? It looks good to me but I want to make sure this meets a minimum criteria for you, and won't introduce any regressive behaviour. If you think this good I'll merge then take a look at #350.

[GeorgeTaveras1231]

@keithamus I would only change this to fall back gracefully if users are using the types@{version} conditions because of what I called out in my previous comment.

This could be handled easily with small impact to scope in two ways

1. Don't apply types sort if the object includes any key matching the pattern types@{version}.

2. Move all types@{version} above types and keep them in their defined order.

This is the only thing I see possibly introducing regression issues for users. Otherwise, this is ok. 👌

[fisker]

Versioned type conditions support added. b73d970

@GeorgeTaveras1231