-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[@mantine/core] Fix error: expression produces a union type that is too complex to represent #6970
Conversation
814bec4
to
97e4f68
Compare
Fortunately the solution is not that easy. Even though It seems that the conditional type reduces the burden on the compiler, the Props type no longer being intersected, means that the resulting type is no longer structurally equal to anything it is matched with. That's why we are seeing these errors. So this made me think, if we should actually look in a different direction for this solution? I understand we might have to intersect with Props, but we likely need a different constraint in the conditional type to fix the problem then. |
I do not have any issues with createPolymorphicComponent types, it does not display the error above for me. You can find a working example here – https://codesandbox.io/p/sandbox/mantine-react-template-forked-q7scwl?file=%2Fsrc%2FApp.tsx%3A23%2C1 |
You can reproduce the error by extending MyButtonProps from StackProps, using "jsxImportSource": "@emotion/react" in tsconfig.json. This error is not related to anything specifically, as it just happens because it crosses the limits of the TS compiler (100k branches i believe). |
The issue is quite specific for emotion library, which is not recommended to be used with Mantine. If you still want to use emotion, then you can follow the guide, which demonstrates usage without changing TypeScript settings. I'd rather not change the logic of polymorphic component types – it will definitely break things. |
I know the docs recommend against using mantine with emotion, but this is not related to any external libraries specifically. I think users should be free to choose, so my take on this is that mantine should prevent any compatibility issues as much as possible. However, if that turns out to be hard in practice (which it seems), i would agree with you to avoid significant changes (unless someone wants to dedicate the time to it). Regarding your suggestions, i might consider changing the approach (currently still using the css prop, but with serialized styles co-located outside of the components for stable references). |
8b80051
to
126f069
Compare
Alright, i have good news. The problem seems to occur because the type resolution within the context of the |
126f069
to
a72ce71
Compare
Thanks for the research, I'd still prefer to leave current types as is to avoid accidental breaking changes – these types are used by the majority of Mantine components and even small changes might have a great impact. |
I understand your hesitance, even though i am quite confident there won't be any regressions, as ComponentProps is itself already an intersection of Props in any possible case. You can see this yourself if you just follow the generics that represent the type Props. So adding So i'd say, let's just take our lessons here. Given the minimal changes, i can live with a patch. I'll change the title of this PR for future readers, if they happen to come across the same error. |
Prevents TypeScript from showing the error: "Expression produces a union type that is too complex to represent.", when props are extended from interfaces such as StackProps and provided as explicit type to createPolymorphicComponent.