Skip to content

fix(useOverlay): support infering close argument from complex emits #4414

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

Open
wants to merge 4 commits into
base: v3
Choose a base branch
from

Conversation

Teages
Copy link

@Teages Teages commented Jun 28, 2025

πŸ”— Linked issue

Resolves #4413

upstream issue: microsoft/TypeScript#32164

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

The PR added the support of infering close argument from complex emits, so that we can get the result type of lazy components.

const lazyDialog = overlay.create(LazyDialog)
const dialog = lazyDialog.open()
const result = await dialog.result
//    ^ now it is not `never`

Verify the changes in TypeScript Playground

The PR supports 16 more overloads, I think that is enough.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly. (N/A)

@Teages Teages requested a review from benjamincanac as a code owner June 28, 2025 07:49
Copy link

pkg-pr-new bot commented Jun 28, 2025

npm i https://pkg.pr.new/@nuxt/ui@4414

commit: 77686cb

@genu
Copy link
Member

genu commented Jun 28, 2025

@Teages This is a very strange workaround. Is this necessary to have type inference for lazy loaded components?

@Teages
Copy link
Author

Teages commented Jun 29, 2025

@Teages This is a very strange workaround. Is this necessary to have type inference for lazy loaded components?

@genu Yes, I think it is pretty useful and lazy compnent is also used in example. The first time I used it I wouldn't have known where I screwed up, if I hadn't looked at the type.

The primary goal is to fix the type of lazy loaded components. But the problem will also happens when it have any other emits with have no argument.

The recursive solution in the original issue may be more regular, but I think it is too complex for the problem we need to resolve.

@benjamincanac benjamincanac requested review from genu and removed request for genu June 30, 2025 08:04
@genu
Copy link
Member

genu commented Jul 2, 2025

@Teages I took some time to try to replicate the problem, but I haven't been able to identify this edge case. can you share how you are overloading the emits?

Do you mean something like this?

const emit = defineEmits<{
  (event: 'close', result: string): void
  (event: 'close', result: number): void
  (event: 'close', result: boolean): void
  (event: 'close', result: { message: string, code: number }): void

}>()

@Teages
Copy link
Author

Teages commented Jul 2, 2025

@genu Did you look into the playground I provided? Before the PR / After this PR

Something like this will break the current type:

const emit = defineEmits<{
  (event: 'close', result: string): void
  (event: 'hydrated'): void // added by lazy component
}>()

@Teages Teages force-pushed the fix/use-overlay-infer-close-type-from-complex-emits branch from b632a18 to 653146b Compare July 2, 2025 18:56
@Teages
Copy link
Author

Teages commented Jul 2, 2025

oops I chose the wrong way to sync to the base. nothing edited in the prev force push

@genu
Copy link
Member

genu commented Jul 2, 2025

Thanks @Teages That makes better sense now.

I've added a comment block to explain a little bit the background behind this and included a link to the original typescript discussion.

Feel free to make any more changes.

I think its good to go πŸ‘ πŸš€

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 this pull request may close these issues.

useOverlay can't infer the result type of lazy load components
2 participants