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

Support TypeScript Node16/NodeNext module resolution #6616

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

robwalch
Copy link
Collaborator

@robwalch robwalch commented Aug 5, 2024

This PR will...

Let TypeScript find the corresponding TypeScript definition file (.d.mts and.d.ts) by extension substitution for ESM and UMD library exports.

Why is this Pull Request needed?

This is to satisfy https://arethetypeswrong.github.io/?p=hls.js

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Resolves #6612

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@aaronadamsCA
Copy link

aaronadamsCA commented Aug 5, 2024

@robwalch, I've seen other libraries just copy the definition file, so yes, I think this approach should work.

However, you'll either need to rename the definition files without the .js in them so TypeScript can automatically resolve them, or just be explicit about the filenames:

  "exports": {
    ".": {
      "import": {
        "types": "./dist/hls.js.d.mts",
        "default": "./dist/hls.mjs"
      },
      "default": {
        "types": "./dist/hls.js.d.ts",
        "default": "./dist/hls.js"
      }
    },
    "./dist/*": "./dist/*",
    "./package.json": "./package.json"
  },

Also note you can npm pack a build and upload it to https://arethetypeswrong.github.io/ to check if the types resolve correctly before publishing any changes.

@robwalch robwalch force-pushed the task/satisfy-attw branch from fe92506 to c41b049 Compare August 6, 2024 21:50
@robwalch
Copy link
Collaborator Author

robwalch commented Aug 6, 2024

I renamed the definition files, with the default output as "hls.d.ts" and the copy task creating "hls.d.mts" and "hls.js.d.ts". The last one is for backwards compatibility with direct links to the original definition file name.

@robwalch
Copy link
Collaborator Author

robwalch commented Aug 8, 2024

@aaronadamsCA

The latest changes pass all arethetypeswrong checks. Let me know if you spot anything else or if this looks good to merge.

@aaronadamsCA
Copy link

@robwalch This looks good to me! +1 to addition by subtraction.

The TypeScript 4.7 release notes confirm "types" was never necessary unless the definition files were in a different location, so as far as I can see this change shouldn't introduce any new problems.

@robwalch robwalch merged commit b890921 into master Aug 13, 2024
16 checks passed
@robwalch robwalch deleted the task/satisfy-attw branch August 13, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support TypeScript Node16/NodeNext module resolution
2 participants