move JSX namespace into preact one #1448
Conversation
|
@just-boris It seems like it might be a little while before we could release this since it breaks TS <3.4, right? |
|
It's not a good sample size but from my experience TS users don't take long to upgrade. |
|
FYI: TypeScript 3.4 was released just a few days ago 🎉 |
I will do extra testing to see how exactly it breaks older TS and if there are any possible workarounds (for example, various options in
Typescript minor releases contain breaking changes, so users are advised to lock version in dependencies. So, onboarding on the new version would require an effort from developers. So, it is worth to consider possible impact of shipping this change. I will check if this change fixes more things than breaks and update the PR. |
|
Updated the PR, added an extra test for this scenario. I have also checked how this works under different versions of Typescript. The above-mentioned error (microsoft/TypeScript#30160) is happening only when you are using Preact and JSX directly. But if you have That said, I believe this change brings more benefits than opens potential issues. Moreover, as soon as change gets released as an alpha version, it should be fine to require the newest typescript to make it work. |
| unknownProperty: string | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This snippet simulates what @types/react does. Will you be interested in having the real React typings as a dev-dependency instead of this mock?
|
I recently ran into a case where some types are actually different, onDoubleClick in react is onDblClick in preact, if i was using react types i would have never found the cause |
Yes this is normalized in preact/compat but in preact the native events are used. |
|
@marvinhagemeister @developit do you need anything else from me to get this PR merged? |
|
@just-boris The PR is awesome and will be merged soon 👍 We were just more focused on getting |
When using both `preact` and `react` in typescript projects, the type checker gets conflicting type definitions because both projects declare global JSX definitions. Typescript since 2.8 has recommended libraries to internally namespace `JSX` to avoid conflicting definitions. See factory functions: https://www.typescriptlang.org/docs/handbook/jsx.html#factory-functions. Preact 10.x already switched to internally namespaced JSX. See preactjs/preact#1448
closes #1036. Still requires some work, but open for initial feedback.
This change fixes issues when preact is used together with
@types/reactin dependencies. Conflicting JSX namespace definitions confuse typescript. Here is a demo project, where I tested integration with react typings: https://github.com/just-boris/preact-mixImplementing the approach described in the related typescript issue: microsoft/TypeScript#23762 (comment)
Not yet ready to merge, because it requires the fix in typescript to be released microsoft/TypeScript#30160.