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(vue): add id props and improve state exposition #918

Merged
merged 3 commits into from May 19, 2023

Conversation

Shyrro
Copy link
Collaborator

@Shyrro Shyrro commented May 8, 2023

BREAKING CHANGE: Update exposed context types and remove unnecessary expose calls

This PR contains :

@vercel
Copy link

vercel bot commented May 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
ark-website ✅ Ready (Inspect) Visit Preview May 19, 2023 3:07pm

@cschroeter
Copy link
Member

"id": {
    "type": "string",
    "isRequired": true,
    "description": "The unique identifier of the machine."
  },

The generated types, in this case the accordion are not 100% correct. The id should be optional. We achieve this in React / Solid doing sth like this:

export type UseAccordionProps = Optional<accordion.Context, 'id'>

@jiblett1000
Copy link

jiblett1000 commented May 16, 2023

Hey ya'll. Just wondering if this PR is taking into account the recent release of Vue 3.3 and specifically being able to use imported types in SFC? I just upgraded and the exported types from Ark don't seem to be compatible for some reason.

vuejs/core#8083

Just curious as this could be a huge time saver as I wouldn't need to re-write all of the props in my custom components, but just import an extend the ones from Ark instead.

@Shyrro
Copy link
Collaborator Author

Shyrro commented May 16, 2023

Hey @jiblett1000

This PR is not related, as the SFC types are only available under SFC's. We use TSX to build our Vue components so we don't have the same issues as we can already use Typescript how we want.

The exported types are pure typescript so you can reuse them in your own defineProps if needed.

Could you elaborate a little bit more on what you mean by "exportes types don't seem compatible for some reason", what errors do you have etc?

Thanks

@jiblett1000
Copy link

Hey @Shyrro I'm happy to create an issue for this if need be. This is where I'm importing / using the type:

import type { NumberInputProps } from "@ark-ui/vue";

interface Props {
  label: string;
  id?: string;
  paddingRight?: number;
  paddingLeft?: number;
}

const props = defineProps<NumberInputProps & Props>();

I modified an existing stackblitz to show the error.... although, I'm getting a different error there than I am locally.

https://stackblitz.com/edit/nuxt-starter-3jzcrb?file=components%2FChildDialog.vue

Shyrro and others added 3 commits May 19, 2023 17:02
BREAKING CHANGE: Update exposed context types and remove unnecessary expose calls

Signed-off-by: Shyrro <zsahmane@gmail.com>
Signed-off-by: Shyrro <zsahmane@gmail.com>
@Shyrro Shyrro merged commit 8280f9e into main May 19, 2023
7 checks passed
@Shyrro Shyrro deleted the fix/addIdsInComponents branch May 19, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants