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

Vue3 custom element prop type number is a string in production. #8987

Closed
SavkaTaras opened this issue Aug 16, 2023 · 17 comments · Fixed by #8989
Closed

Vue3 custom element prop type number is a string in production. #8987

SavkaTaras opened this issue Aug 16, 2023 · 17 comments · Fixed by #8989
Labels
🐞 bug Something isn't working

Comments

@SavkaTaras
Copy link

Vue version

3.3.4

Link to minimal reproduction

https://github.com/SavkaTaras/vue-cue-issues

Steps to reproduce

npm install
npm run build
npm run dev

What is expected?

We import vue component and the same custom element below.

The custom element typeof should be number. The sum of 5.5 + 0.5 should be 6.

What is actually happening?

Custom element typeof returns a string.
Sum is 5.50.5.

System Info

System:
    OS: macOS 13.5
    CPU: (12) x64 Apple M2 Pro
    Memory: 201.20 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 14.20.0 - /usr/local/bin/node
    Yarn: 1.22.19 - ~/.npm-global/bin/yarn
    npm: 6.14.17 - /usr/local/bin/npm
  Browsers:
    Chrome: 114.0.5735.289
    Safari: 16.6
  npmPackages:
    vue: 3.3.4 => 3.3.4

Any additional comments?

Hello vue team,
I hope you are doing well.

@baiwusanyu-c could you please take a look at this issue when you have a moment?

Thank you so much guys,
Taras

@baiwusanyu-c
Copy link
Member

I can't seem to reproduce the problem in the playground

@baiwusanyu-c
Copy link
Member

I reproduced the problem, he only appears in some specific syntax

<template>
	<div class="component-a">
		<p>foo value: {{ props.foo }}</p>
		<p>typeof foo: {{ typeof props.foo }}</p>
		<p>Sum: {{ props.valOne + props.valTwo }}</p>
		<!-- <ComponentB /> -->
	</div>
</template>

<script lang="ts" setup>

interface Props {
  foo?: number;
	valOne: number;
	valTwo: number;
}

const props = withDefaults(defineProps<Props>(), {
	foo: 1,
	valOne: 2.5,
	valTwo: 0.5,
});
</script>

@SavkaTaras
Copy link
Author

SavkaTaras commented Aug 16, 2023

I see what you did. Is it possible to test per these docs:

Props Default Values section:

https://vuejs.org/guide/typescript/composition-api.html#typing-component-props

Thank you

@SavkaTaras
Copy link
Author

We really enjoy setting the type with defaults. When using as vue component, typescript provides some valuable suggestions. vue-tsc helps during the type-check command. But when compiling to custom element, it doesn't seem to work.

Sum: {{ props.valOne + props.valTwo }}

only works correctly if no value is passed to valOne and valTwo. Because the default gets defined correctly (from dist file):
import { defineComponent as m, openBlock as l, createElementBlock as r, createElementVNode as t, toDisplayString as s, defineAsyncComponent as _, createVNode as u, unref as i, defineCustomElement as p } from "vue";
const d = { class: "component-a" }, C = /* @__PURE__ */ m({
  __name: "ComponentA",
  props: {
    foo: { default: 1 },
    valOne: { default: 2.5 },
    valTwo: { default: 0.5 }
  },

I would almost think that this withDefaults option doesn't enforce the number type.

Thank you again!

@baiwusanyu-c
Copy link
Member

I think this is a bug. The compiler does not compile the type information of the props under the product environment, but the Custom Element does not do special processing in this situation in Runtime

cc/@sxzz

@LinusBorg
Copy link
Member

@baiwusanyu-c This is a bug, but your PR won't comletely solve it. when defining a prop like this, without a default:

<script setup lang="ts">
    const props = defineProps<{
      foo: number
    }>()
</script>
<template>
<div>{{ foo + bar }}</div>
</template>

We end up with this in production:

const __sfc__ = _defineComponent({
  __name: 'Comp',
  props: {
    foo: {}
  },
  setup(__props) {

That's a behavior of definePropswhen using it with TS props definition style, the reasoning being that we don't need runtime props defitions when we provide types for the consumer.

But when using such a component as a custom element, we get the reported problem, as now the custom element has no way of knowing, at runtime, that it needs to cast this attribute as a number.

This likely needs a solution at the compiler level. Either the stripping of props types needs to be removed completely, or it needs to be deactivated when compiling a component as a custom component (*.ce.vue)

@LinusBorg LinusBorg added the 🐞 bug Something isn't working label Aug 18, 2023
@baiwusanyu-c
Copy link
Member

You are right, thank you for your help, let me think about how to do it🧐

@SavkaTaras
Copy link
Author

Hey @LinusBorg @baiwusanyu-c ,

Do you guys think it's worth adding a Number() wrapper around any number value? It would enforce the number conversion. Since the user is clearly defining a number, the enforcement could be helpful.

Thanks

@baiwusanyu-c
Copy link
Member

Hey @LinusBorg @baiwusanyu-c ,

Do you guys think it's worth adding a Number() wrapper around any number value? It would enforce the number conversion. Since the user is clearly defining a number, the enforcement could be helpful.

Thanks

This case of customElement can be specially handled in the compiler, I made some changes in pr, it is not perfect, I don't have time over the weekend, I will improve it next week

@SavkaTaras
Copy link
Author

@baiwusanyu-c no worries at all. I saw the PR changes. I was just thinking maybe that could help.

Thank you again

@funkso2010
Copy link

Hello @baiwusanyu-c & @sxzz ,
I hope you are doing well.

Do you guys have any estimates when the new version of vue3 is going to be released?

Thank you!

@SavkaTaras
Copy link
Author

Hello Vue team.

Do you guys have any idea when the fix would be available for this issue (released)?

Thank you.

@SavkaTaras
Copy link
Author

Hello @LinusBorg , @sxzz , @baiwusanyu-c ,
I hope you are well.

I'm still getting the issue after updating "@vitejs/plugin-vue": "^4.5.2".

Please let me know if you guys can reproduce. I pushed updated package versions into the repo.

Thank you,
Taras

@LinusBorg
Copy link
Member

that's because we haven't had a new patch release for Vue since we merged #8989

the changes in the vite plugin are a prerequisite for that.

@sxzz
Copy link
Member

sxzz commented Dec 8, 2023

[email protected] is released now.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants