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

Use TS typings provided by @types/hotwired__turbo #17654

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

myabc
Copy link
Contributor

@myabc myabc commented Jan 19, 2025

⚠️ We may want to wait until DefinitelyTyped/DefinitelyTyped#71699 is merged before reviewing/merging this PR.

Ticket

N/A

What are you trying to accomplish?

  • Reducing the amount of code we need to maintain.
  • Improving the accuracy of typings.

Screenshots

What approach did you choose and why?

Although I went ahead and created this PR, it is worth mentioning that the typings provided by @types/hotwired__turbo are far from complete. We might want to invest a bit of time in making some of our own shims available upstream, e.g. frontend/src/typings/shims.d.ts

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

myabc added 2 commits January 19, 2025 19:42
Replaces our own custom typings for Turbo elements and events.
@myabc myabc requested a review from cbliard January 19, 2025 22:51
myabc added 2 commits January 19, 2025 21:27
This allows TypeScript to infer the correct return type for methods that
accept a selector string argument, such as `document.querySelector` and
`Element.closest`.
@myabc myabc force-pushed the housekeeping/use-@types/hotwired__turbo-ts-typings branch from f439c6c to 1673126 Compare January 20, 2025 00:27
Comment on lines 36 to 40
declare global {
interface HTMLElementTagNameMap {
'turbo-frame':FrameElement
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an upstream PR for this: DefinitelyTyped/DefinitelyTyped#71699

@oliverguenther
Copy link
Member

Nice @myabc 👍 +1 for upstreaming our modifications

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

Successfully merging this pull request may close these issues.

2 participants