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(cli): missing props/emits when doesn't enable typescript #105

Merged
merged 8 commits into from
Nov 2, 2023

Conversation

Dunqing
Copy link
Collaborator

@Dunqing Dunqing commented Oct 7, 2023

close: #62

Since the author left it unaddressed for a long time, and in case Javascipt users long time to use, I publish a detypes package to temporarily fix the problem.

@zernonia
Copy link
Member

Thanks for the hard work @Dunqing ! Could you add a simple test case for this transformation? 😁

@Dunqing
Copy link
Collaborator Author

Dunqing commented Oct 22, 2023

Thanks for the hard work @Dunqing ! Could you add a simple test case for this transformation? 😁

Sure, I added

@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Oct 22, 2023

Tested your branch, pnpm build cli and pnpm start add button

Button.vue

<script setup>
import { buttonVariants } from '.'
import { cn } from '@/lib/utils'

withDefaults( // <=== this API is typescript related
  defineProps({
    variant: { type: null, required: false },
    size: { type: null, required: false },
    as: { type: String, required: false, default: 'button' },
  }),
  {
    as: 'button',
  },
)
</script>

<template>
  <component
    :is="as"
    :class="cn(buttonVariants({ variant, size }), $attrs.class ?? '')"
  >
    <slot />
  </component>
</template>

Also tested with npm link for using cli outside of the project in fresh vite template-vue

> shadcn-vue add alert-dialog
√ Ready to install components and dependencies. Proceed? ... yes
- Installing components...Cannot read properties of undefined (reading 'sys') // error

When jsconfig.json is not present

ENOENT: no such file or directory, stat 'vue-template/jsconfig.json' // error

@Dunqing
Copy link
Collaborator Author

Dunqing commented Oct 25, 2023

Tested your branch, pnpm build cli and pnpm start add button

Button.vue

<script setup>
import { buttonVariants } from '.'
import { cn } from '@/lib/utils'

withDefaults( // <=== this API is typescript related
  defineProps({
    variant: { type: null, required: false },
    size: { type: null, required: false },
    as: { type: String, required: false, default: 'button' },
  }),
  {
    as: 'button',
  },
)
</script>

<template>
  <component
    :is="as"
    :class="cn(buttonVariants({ variant, size }), $attrs.class ?? '')"
  >
    <slot />
  </component>
</template>

Also tested with npm link for using cli outside of the project in fresh vite template-vue

> shadcn-vue add alert-dialog
√ Ready to install components and dependencies. Proceed? ... yes
- Installing components...Cannot read properties of undefined (reading 'sys') // error

When jsconfig.json is not present

ENOENT: no such file or directory, stat 'vue-template/jsconfig.json' // error

Thank you for testing this PR! Yes, I forgot the case with withDefautls, I fixed it in Dunqing/detype@add6e46.

@Dunqing
Copy link
Collaborator Author

Dunqing commented Oct 25, 2023

@sadeghbarati Can you take a look at it again? Thank you

@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Oct 25, 2023

shadcn-vue add button

<script setup>
import { buttonVariants } from ".";
import { cn } from "@/lib/utils";

defineProps({
  variant: { type: null, required: false },
  size: { type: null, required: false },
  as: { type: String, required: false, default: "button" },
});
</script>

<template>
  <component
    :is="as"
    :class="cn(buttonVariants({ variant, size }), $attrs.class ?? '')"
  >
    <slot />
  </component>
</template>

shadcn-vue add alert-dialog

shadcn-vue add alert-dialog

√ Ready to install components and dependencies. Proceed? ... yes
- Installing components...[@vue/compiler-sfc] No fs option provided to `compileScript` in non-Node environment. File system access is required for resolving imported types.

anonymous.vue
2  |  import { type AlertDialogEmits, type AlertDialogProps, AlertDialogRoot, useForwardPropsEmits } from 'radix-vue'
3  |
4  |  const props = defineProps<AlertDialogProps>()
   |                            ^^^^^^^^^^^^^^^^
5  |  const emits = defineEmits<AlertDialogEmits>()

@Dunqing
Copy link
Collaborator Author

Dunqing commented Oct 25, 2023

√ Ready to install components and dependencies. Proceed? ... yes

  • Installing components...[@vue/compiler-sfc] No fs option provided to compileScript in non-Node environment. File system access is required for resolving imported types.

anonymous.vue
2 | import { type AlertDialogEmits, type AlertDialogProps, AlertDialogRoot, useForwardPropsEmits } from 'radix-vue'
3 |
4 | const props = defineProps()
| ^^^^^^^^^^^^^^^^
5 | const emits = defineEmits()

Oh, This case seems hard to fix. Even if I pass fs in, it doesn't work, I can't get radix-vue as the real path.

@Dunqing
Copy link
Collaborator Author

Dunqing commented Nov 2, 2023

Now it's supported! Do you @sadeghbarati have time to test again? I have tested it, Looks good!
image

@sadeghbarati
Copy link
Collaborator

Sure thanks for your efforts ❤️

@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Nov 2, 2023

@Dunqing

All good ❤️ but this 👇 which is happening in some component

- const defineProps
+ const props = defineProps

@Dunqing
Copy link
Collaborator Author

Dunqing commented Nov 2, 2023

@Dunqing

All good ❤️ but this 👇 which is happening in some component

- const defineProps

+ const props = defineProps

Thanks, I noticed that the test cases had this problem as well

@Dunqing
Copy link
Collaborator Author

Dunqing commented Nov 2, 2023

@Dunqing

All good ❤️ but this 👇 which is happening in some component

- const defineProps
+ const props = defineProps

Fixed

@sadeghbarati sadeghbarati requested a review from zernonia November 2, 2023 14:10
@zernonia
Copy link
Member

zernonia commented Nov 2, 2023

Absolute amazing work @Dunqing ! Thanks for taking on this difficult challenge!
@sadeghbarati thanks for the review!

Copy link
Member

@zernonia zernonia left a comment

Choose a reason for hiding this comment

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

LGTM! ❤️

@zernonia zernonia merged commit a4774ff into unovue:dev Nov 2, 2023
1 of 2 checks passed
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.

[Bug]: Javascript has empty props/emits
3 participants