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

Fix missing types from exports #332

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JAD3N
Copy link

@JAD3N JAD3N commented Nov 22, 2024

No description provided.

@lupomontero lupomontero self-requested a review November 25, 2024 18:26
@lupomontero lupomontero self-assigned this Nov 25, 2024
@lupomontero lupomontero linked an issue Nov 25, 2024 that may be closed by this pull request
@lupomontero
Copy link
Owner

Hi @JAD3N, thanks for the PR 💪

I'm not that familiar with TypeScript 🙈, and I'm a bit confused by this change 😰

Your change adds exports.*.types to package.json. Something like:

{
  "exports": {
    ".": {
      "types": "./types/index.d.ts",
      "import": "./dist/psl.mjs",
      "require": "./dist/psl.cjs"
    }
  }
}

However, in the TypeScript docs I can only find a mention to types (which is already present in psls package.json).

On the other hand, doing a bit of research, I found some info in the release notes for [email protected] as well as an "official" blog post announcing that same version. In both the following kind of setup is suggested.

{
  "exports": {
    ".": {
      "import": {
        "types": "./types/index.d.ts",
        "default": "./dist/psl.mjs"
      },
      "require": {
         "types": "./types/index.d.ts",
        "default":  "./dist/psl.cjs"
      }
    }
  }
}

Can you shed some light on this? Are you familiar with the stuff mentioned above? I'd really like to understand the implications in depth before we merge the PR.

@JAD3N
Copy link
Author

JAD3N commented Nov 25, 2024

Hi @lupomontero,

I believe the exports.*.types property is a "community condition definition" which TypeScript uses for retrieving typings of specific conditional exports of imported packages/modules when type-checking.

This depends on the version of TypeScript and the configured module resolution. This link here clarifies that the newer "bundler" module resolution utilizes conditional exports, so we would want to keep the current types and add the exports.*.types as well for full compatibility.

I'm not too sure on whether it needs separate typing files for ESM & CJS, I'm using mostly ESM nowadays, so I've not encountered an issue regarding this. It may be worth adding an additional typings file for CJS as well, I think that would just change things to export via module.exports , but I'm unsure which one you would use in the root types property in the package.json.

Edit: From what I can find out here it seems that the existing and this PR's typing file appear to be compatible with both but more research might be needed to clarify.

Edit 2: Another link I've found that might be more useful: https://www.typescriptlang.org/docs/handbook/modules/appendices/esm-cjs-interop.html#allowsyntheticdefaultimports-and-esmoduleinterop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing types from package exports
2 participants