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

TypeError of Dialog and Dialog.Title from the official example #2117

Closed
xsjcTony opened this issue Dec 22, 2022 · 8 comments · Fixed by #2242
Closed

TypeError of Dialog and Dialog.Title from the official example #2117

xsjcTony opened this issue Dec 22, 2022 · 8 comments · Fixed by #2242

Comments

@xsjcTony
Copy link

xsjcTony commented Dec 22, 2022

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.7.7

What browser are you using?

Chrome v108

Reproduction URL

https://headlessui.com/react/dialog 's main example

My screenshots below used totally the same copy-pasted code without any modification

Describe your issue

TypeScript version: ^4.9.4,
Vite: ^4.0.2,
All packages' versions are the latest

className throwing TypeError if as has been set (it works if as is not defined)
image
image

tsc --noEmit result
image

@xsjcTony
Copy link
Author

I found the problem, which is because I originally set

allowSyntheticDefaultImports to false in tsconfig.json

After I changed it to true, the issue is gone.


But I still want to use false for that value, any solution on that?

@RobinMalfait
Copy link
Member

Hey! Thank you for your bug report!
Much appreciated! 🙏

Can you create a minimal reproduction repo and attach it to the issue?

@xsjcTony
Copy link
Author

xsjcTony commented Jan 5, 2023

@RobinMalfait Hey, here it is
https://stackblitz.com/edit/vitejs-vite-kksxtq?file=src/headlessuiDemo/Demo.tsx

So basically it's because I set allowSyntheticDefaultImports to false and that's exactly what I want, but seems like headless not UI does not support that.
I reckon the type system is using some allowSyntheticDefaultImports-only import syntax so that's why. Please inspect, thanks a lot.

@xsjcTony
Copy link
Author

xsjcTony commented Jan 25, 2023

@RobinMalfait Hey, sorry to bother you, but I can see the "Needs more info" tag is still there, not sure if my update has been seen.

@xsjcTony
Copy link
Author

xsjcTony commented Jan 25, 2023

I think the cause might be dts file used the syntax that requires allowSyntheticDefaultImports to be turned on, e.g.

import React from 'react'

Snipaste_2023-01-25_23-22-51

Not sure if there will be a fix to import * as React from 'react' or use named import instead for that.

RobinMalfait added a commit that referenced this issue Feb 2, 2023
We use named imports, but we have to import `React` itself as well for
JSX because it compiles to `React.createElement`. We could get rid of
our own JSX and use it directly, or we can use this `import * as React
from 'react'` syntax.

This fixes an issue for people using `allowSyntheticDefaultImports: false` in TypeScript.

Fixes: #2117
RobinMalfait added a commit that referenced this issue Feb 2, 2023
* use the `import * as React from 'react'` pattern

We use named imports, but we have to import `React` itself as well for
JSX because it compiles to `React.createElement`. We could get rid of
our own JSX and use it directly, or we can use this `import * as React
from 'react'` syntax.

This fixes an issue for people using `allowSyntheticDefaultImports: false` in TypeScript.

Fixes: #2117

* update changelog
@RobinMalfait
Copy link
Member

This should be fixed, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.

@RobinMalfait RobinMalfait reopened this Feb 6, 2023
@RobinMalfait
Copy link
Member

The changes applied to fix this issue broke some existing applications. Re-opened the issue for now so that we can keep track of it. If we can't find a solution that both fixes this issue and keeps the existing applications from working than this issue will be closed with a "Won't fix".

@RobinMalfait RobinMalfait removed their assignment Feb 7, 2023
@RobinMalfait
Copy link
Member

I don't immediately see a way for us to fix this without breaking existing applications. Going to close it for now, but maybe pick this up later when necessary.

For now you have 2 options I think:

  • Use the specific insiders version that did have support for it (even though this would lack newer features and bug fixes)
  • Or use the allowSyntheticDefaultImports flag and set it to true in tsconfig.json. I believe this is also the default in various scenarios already so dropping it might be enough.

Sorry for the inconvenience, hope this helps!

@RobinMalfait RobinMalfait closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2023
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 a pull request may close this issue.

2 participants