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

perf: Avoid doing type gymnastics with JSX.IntrinsicElements #3259

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

johnsoncodehk
Copy link
Member

@johnsoncodehk johnsoncodehk commented Jun 8, 2023

close #3226, close #3148, close #3130, close #2533

First I wasn't able to actually reproduce the problem, so please let me know if it doesn't solve the above closed issue. Thanks to @Fuzzyma for helping to investigate, I think the problem is most likely happening in #2685.

In this PR we no longer do type gymnastics with JSX.IntrinsicElements, so TS type evaluation may be faster.

But it also makes the template unable to separate JSX.IntrinsicElements and components types, which affects DX in some cases, but I think it is a reasonable trade-off, and it is too expensive to trade performance for it.

@Fuzzyma
Copy link
Contributor

Fuzzyma commented Jun 8, 2023

That sounds awesome! Thank you so much for digging into it (especially because I didn't provide any reasonable reproduction 😅)

@johnsoncodehk johnsoncodehk merged commit c667268 into master Jun 8, 2023
@johnsoncodehk johnsoncodehk deleted the fast-jsx branch June 8, 2023 16:50
@rchl
Copy link
Collaborator

rchl commented Jun 8, 2023

But it also makes the template unable to separate JSX.IntrinsicElements and components types

Can you expand a bit on that?
Are JSX.IntrinsicElements all the native DOM elements?
Does this PR affect the behavior of handling unknown attributes/props on components? For example I think it would be fine to allow any custom attribute on a native DOM element but I would expect an error to be raised when setting unknown prop/attribute on a component (with some exceptions like class and other).

@johnsoncodehk
Copy link
Member Author

For example, for the following code:

<template>
  <img alt="Vue logo" src="./assets/logo.png" />
</template>

<script lang="ts" setup>
import img from './components/HelloWorld.vue';
</script>

<img> type will be JSX.IntrinsicElements['img'] & typeof import('./components/HelloWorld.vue')['default'].

So you just need to avoid component name same with JSX.IntrinsicElements properties.

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