Skip to content

Conversation

@andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Sep 21, 2022

package.json exports is a superset of the features supported by typesVersions, but currently typesVersions is consulted first. This prevents TypeScript from recognizing the exports for a path that also matches a typesVersions path, which is a big problem if the package wants to use conditional exports that benefit --moduleResolution nodenext and typesVersions for --moduleResolution node. As far as I can tell, this was just an oversight, as no existing tests broke when I changed the order.

Thanks to @fictitious for noticing at andrewbranch/example-subpath-exports-ts-compat#1

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a types@>=4 condition or the like would be useful as a test alongside the typesVersions object, too, if you're up for adding another test case.

@andrewbranch andrewbranch merged commit 221cf55 into microsoft:main Sep 22, 2022
@andrewbranch andrewbranch deleted the bug/exports-should-block-types-versions branch September 22, 2022 00:21
@Andarist
Copy link
Contributor

This probably needs a big call out in the docs in the typesVersions section (and somewhere in the moduleResolution stuff too).

@andrewbranch andrewbranch added the Breaking Change Would introduce errors in existing code label Sep 22, 2022
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 23, 2022

How's this for the release notes?

Previously, TypeScript incorrectly prioritized the typesVersions field over the exports field when resolving through a package.json under --moduleResolution node16.
If this change impacts your library, you may need to add types@ version selectors in your package.json's exports field.

  {
      "type": "module",
      "main": "./dist/main.js"
      "typesVersions": {
          "<4.8": { ".": ["4.8-types/main.d.ts"] },
          "*": { ".": ["modern-types/main.d.ts"] }
      },
      "exports": {
          ".": {
+             "types@<4.8": "4.8-types/main.d.ts",
+             "types": "modern-types/main.d.ts",
              "import": "./dist/main.js"
          }
      }
  }

@Andarist
Copy link
Contributor

q: shouldn't the presented diff only add the types@<4.8 key? With the logic from this PR in place - are typesVersions still handled with --moduleResolution node16 if there no types-related key in package.json#exports? Or are those completely ignored with this moduleResolution?

@andrewbranch
Copy link
Member Author

The presence of exports blocks any non-exports keys from contributing to package resolution. That has always been the intention and has been the case for the types and main fields.

@weswigham
Copy link
Member

    "exports": {
         ".": {
            "import": {
                "types@<4.8": "4.8-types/main.d.ts",
                "types": "modern-types/main.d.ts",
                "default": "./dist/main.js"
             }
         }
     }

Is how you should shape that export object, to accommodate types for a require condition or similar.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team Breaking Change Would introduce errors in existing code For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants