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

fix(deps): uses defineSlots macro over useSlots #3444

Merged

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Jan 21, 2025

Required for #3336

I got a bit more time to look at #3336 and it looks like Vue folks are moving more and more to macro-based defineSlots for defining slots.

Moving everything to use defineSlots worked in most areas and I only had to define the types also for areas where we actually need the type information, otherwise defineSlots is a Record<string, any> (see https://vuejs-language-tools.vercel.app/features/slots#how-to-handle-indeterminate-slot-types)

There 'may' be some other places where it would be advantageous to define our slots as more than Record<string, any> but for the moment this is enough to unblock the upgrade. We can add any more type information if required in later PRs.

Also see https://vuejs.org/api/sfc-script-setup#defineslots

@johncowen johncowen requested a review from a team as a code owner January 21, 2025 17:25
@johncowen johncowen requested review from schogges and removed request for a team January 21, 2025 17:25
@@ -29,21 +29,20 @@
<template
v-for="key in Object.keys(slots)"
:key="key"
#[key]="{ row, rowValue }"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't use rowValue anywhere so I removed it.

Copy link
Contributor

@schogges schogges left a comment

Choose a reason for hiding this comment

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

👍
In #3416 I now also went for defineSlots

@johncowen
Copy link
Contributor Author

In #3416 I now also went for defineSlots

Nice! I guess we could lint for these things (in this case useSlots usage instead of defineSlots) in future. I guess Vue folks might add something at somepoint. Although I can probably think of other things where we need to expose/use an import only in one place (so it needs to be public/exposed) but it shouldn't be used anywhere else.

@schogges
Copy link
Contributor

In #3416 I now also went for defineSlots

Nice! I guess we could lint for these things (in this case useSlots usage instead of defineSlots) in future. I guess Vue folks might add something at somepoint. Although I can probably think of other things where we need to expose/use an import only in one place (so it needs to be public/exposed) but it shouldn't be used anywhere else.

Sounds good 👍 🙂

@johncowen johncowen merged commit 7b6ab0f into kumahq:dependabot/npm_and_yarn/vue-tsc-2.2.0 Jan 22, 2025
11 checks passed
johncowen added a commit that referenced this pull request Jan 22, 2025
Required for #3336

I got a bit more time to look at
#3336 and it looks like Vue folks
are moving more and more to macro-based `defineSlots` for defining
slots.

Moving everything to use defineSlots worked in most areas and I only had
to define the types also for areas where we actually need the type
information, otherwise defineSlots is a `Record<string, any>` (see
https://vuejs-language-tools.vercel.app/features/slots#how-to-handle-indeterminate-slot-types)

There 'may' be some other places where it would be advantageous to
define our slots as more than `Record<string, any>` but for the moment
this is enough to unblock the upgrade. We can add any more type
information if required in later PRs.

Also see https://vuejs.org/api/sfc-script-setup#defineslots

Signed-off-by: John Cowen <[email protected]>
johncowen pushed a commit that referenced this pull request Jan 22, 2025
From #3444

I got a bit more time to look at #3336 and it looks like Vue folks are moving more and more to macro-based `defineSlots` for defining slots.

Moving everything to use defineSlots worked in most areas and I only had to define the types also for areas where we actually need the type information, otherwise defineSlots is a `Record<string, any>` (see https://vuejs-language-tools.vercel.app/features/slots#how-to-handle-indeterminate-slot-types)

There 'may' be some other places where it would be advantageous to define our slots as more than `Record<string, any>` but for the moment this is enough to unblock the upgrade. We can add any more type information if required in later PRs.

Also see https://vuejs.org/api/sfc-script-setup#defineslots

Then also:

---

Bumps [vue-tsc](https://github.com/vuejs/language-tools/tree/HEAD/packages/tsc) from 2.1.10 to 2.2.0.
- [Release notes](https://github.com/vuejs/language-tools/releases)
- [Changelog](https://github.com/vuejs/language-tools/blob/master/CHANGELOG.md)
- [Commits](https://github.com/vuejs/language-tools/commits/v2.2.0/packages/tsc)

---
updated-dependencies:
- dependency-name: vue-tsc
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
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.

2 participants