Skip to content

fix(dialogs/spawnDialog): enhance spawnDialog types #6781

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

Merged

Conversation

mattersj
Copy link
Contributor

@mattersj mattersj commented Apr 15, 2025

☑️ Resolves

  • A next iteration of fix(dialogs/spawnDialog): improve typing #6770 that improves spawnDialog types:
  • Now only components with a declared @close event can be passed to spawnDialog, otherwise an error is thrown on a type level suggesting what went wrong
  • Autocompletion for props
  • Return type is automatically inferred based on a @close event's payload

The implementation passes all the test cases from #6770 (comment)

🖼️ Screenshots

🏚️ Before 🏡 After
B A

🚧 Tasks

  • ...

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport to stable8 for maintained Vue 2 version or not applicable

@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews feature: functions composables, functions, mixins and other non-components labels Apr 15, 2025
@ShGKme ShGKme added this to the v9.0.0-rc.0 milestone Apr 15, 2025
@ShGKme ShGKme requested review from ShGKme and susnux and removed request for ShGKme April 15, 2025 19:35

This comment was marked as off-topic.

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 💙

import { createApp } from 'vue'
/* eslint-disable @typescript-eslint/no-explicit-any */

import { createApp, type Component } from 'vue'
Copy link
Contributor

@ShGKme ShGKme Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to always separate type and non-type imports (not included in the old eslint config, using in this repo).

Suggested change
import { createApp, type Component } from 'vue'
import type { Component } from 'vue'
import { createApp } from 'vue'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, fixed.

Comment on lines +15 to +17
type DialogComponent<T extends Component> = 'onClose' extends keyof ComponentProps<T>
? T
: 'Please provide a Dialog Component that supports `@close` event'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the best never I've ever seen

@ShGKme ShGKme changed the title fix(types): enhance spawnDialog types fix(dialogs/spawnDialog): enhance spawnDialog types Apr 15, 2025
@mattersj mattersj force-pushed the fix/spawnDialog--improvements branch from 2177ab4 to b668872 Compare April 15, 2025 19:50
@ShGKme ShGKme requested review from DorraJaouad and Antreesy April 15, 2025 19:51
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@susnux susnux merged commit 02a15b5 into nextcloud-libraries:main Apr 15, 2025
27 checks passed
@mattersj mattersj deleted the fix/spawnDialog--improvements branch April 15, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: functions composables, functions, mixins and other non-components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants