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: add type declarations for serialize-request #940

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

rowanmanning
Copy link
Member

@rowanmanning rowanmanning commented Feb 19, 2024

This adds manual type declarations for serialize-request which should make it work in most projects. The tests are now passing.

The compromise is this:

  • ✅ In a CommonJS app (non-compiled)
  • ✅ In an ESM app (non-compiled)
  • ✅ In a TypeScript app that's written in ESM (esModuleInterop set to true)
  • ✅ In a TypeScript app that's written in ESM (esModuleInterop set to false)
  • ✅ In a CommonJS app which uses Sucrase
  • ❌ In a TypeScript app that's written in CommonJS (esModuleInterop set to true)
  • ❌ In a TypeScript app that's written in CommonJS (esModuleInterop set to false)

In the ❌ed projects above everything will still work but you won't get type hinting in VSCode and the types will be any. I'm not sure if it's possible to fix this without breaking the other module systems which are more commonly-used.

This adds manual type declarations for serialize-request which should
make it work in most projects. The tests are now passing.
@rowanmanning rowanmanning requested a review from a team as a code owner February 19, 2024 10:53
@rowanmanning rowanmanning merged commit b45d3e4 into main Feb 20, 2024
11 checks passed
@rowanmanning rowanmanning deleted the serialize-request-manual-types branch February 20, 2024 09:41
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.

2 participants