feat(useOverlay): add on method to listen emits from component

[edimitchel]

🔗 Linked issue

Resolved #5233

❓ 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

Introducing this new on method permits to easy bind emits event before opening for let developer to only think about props in create/open/patch methods.

📝 Checklist

• I have linked an issue or discussion.
• I have updated the documentation accordingly.

[pkg-pr-new]

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

commit: 8dde938

[edimitchel]

I forget the API instance documentation + Caveats section miss a warning about the limitation of the composable.

We need to discuss about its limitation is caused by typescript limitations

[edimitchel]

@genu can you take a look?

[genu]

Thanks for this @edimitchel I think it looks good. The on syntax makes better sense and its cleaner.

I think it may be even a good idea to deprecate the old way of handling events. Having multiple ways could be confusing.

What do you think about that?

We could maybe, add a deprecated message for now, if possible, when users try to handle events like this:

const modal = overlay.create(LazyModalExample, {
  props: {
    count: count.value,
    onClose: (value: number) => {
      console.log('Modal closed with value:', value)
    }
  }
})

Otherwise, we could depreciate them in a future version as a breaking change

[genu]

@edimitchel Let me know if you need some help with this

[edimitchel]

Here we are, I made it (sorry for the delay, I didn't take time for doing this small change).
I didn't achieve to add a TS deprecation as it's on props I have to track but this is dynamic and not manageable from our side.

Let me know if you see something wrong.

[genu]

Thanks for the updates @edimitchel

I left som comments.

A bigger issue that we need to address is the breaking change that we should avoid.

For example, this would not work as expected:

const modal = overlay.create(LazyModalExample, {
  props: {
    count: count.value
  }
})

modal.on('close', () => {
  console.log('Modal closed')
})

function openModal() {
  count.value++

  modal.open({ count: count.value })
}

The on callback fires, but the modal doesn't close. See my explanation on the vercel comment above.

[edimitchel]

I'm not sure it's good now but I'm now not very confident with this feature because of TypeScript limitation which will make the on unusable when there is more than 5 props on the component