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

Vue core issue tracker Vue warns of a type check failure #1846

Closed
6 tasks done
suzumejakku opened this issue Apr 15, 2024 · 18 comments · Fixed by #1850 or #1866 · May be fixed by #1856
Closed
6 tasks done

Vue core issue tracker Vue warns of a type check failure #1846

suzumejakku opened this issue Apr 15, 2024 · 18 comments · Fixed by #1850 or #1866 · May be fixed by #1856
Labels
bug Something isn't working

Comments

@suzumejakku
Copy link

suzumejakku commented Apr 15, 2024

Describe the bug

When using BFormRadioGroup, Vue warns of a type check failure for the v-model. It seems to always expect a Boolean. Check the console logs in the StackBlitz reproduction.

Reproduction

https://stackblitz.com/edit/vue-yqphhw?file=src%2FApp.vue

Used Package Manager

yarn

Validations

  • Have tested with the latest version. This is still alpha version and sometime things change rapidly.
  • Follow our Code of Conduct
  • Read the Contributing Guide.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • Check that this is a concrete bug. For Q&A, please open a GitHub Discussion instead.
  • The provided reproduction is a minimal reproducible of the bug.
@suzumejakku suzumejakku added the bug Something isn't working label Apr 15, 2024
Copy link

stackblitz bot commented Apr 15, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@dwgray
Copy link
Contributor

dwgray commented Apr 15, 2024

@VividLemon - this is the same issue that I noted in my description of #1840, sorry if it got lost in the word salad:

On the defineModel issue, it looks like the model isn't getting defined as expected. I believe this may be the same issue that I saw in #1822 - According to the vue docs:

In dev mode, the compiler will try to infer corresponding runtime validation from the types. For example here foo: String is inferred from the foo: string type. If the type is a reference to an imported type, the inferred result will be foo: null (equal to any type) since the compiler does not have information of external files.

In dev mode I'm seeing errors like this: [Vue warn]: Invalid prop: type check failed for prop "modelValue". Expected Boolean, got Array for things like BFormCheckbox And if I move the type declaration of BFormCheckboxProps into BFormCheckboxVue I get an error on the when using defineModel - `Duplicate key 'modelValue'. May cause name collision in script or template tag.

I've actually tried reverting back to useVModel and the tests still fail. Also tried reverting the versions of dependencies in package.json I must be misunderstanding something about how the system builds/works because I would expect the combination of those two actions to get back to passing tests.

Now that these changes are checked in, these warning are showing up in the playground. I get a number of variations on the following when showing BFormCheckbox in the playground:

[Vue warn]: Invalid prop: type check failed for prop "modelValue". Expected Boolean, got Array

Oddly, if I copy the TFormCheckbox.vue code to packages\bootstrap-vue-next\src\App.vue - I'm not seeing the warnings in the hot reload app.

Again, I tried inlining the types into BFormCheckbox.vue, which should fix the issue. I then ran pnpm install and pnpm dev and opened the playground again. Same error. So either I'm totally off base, or something is being cached somewhere in the build process that I don't understand.

@xvaara
Copy link
Contributor

xvaara commented Apr 16, 2024

It's weird. The type is:
Screenshot 2024-04-16 at 13 11 35
but it's read in different order which causes the sfc-compiler to infer Boolean type.
Screenshot 2024-04-16 at 13 11 56
Boolean should be before string.

@VividLemon vuejs/core#10676
Oh, it's vue bug. It's fixed in 3.4.22. We should rebuild new version.

@xvaara
Copy link
Contributor

xvaara commented Apr 17, 2024

I've tracked this down to the line in RadioTypes.ts:
| readonly unknown[] should be | Readonly<unknown[]> I think?

@VividLemon what do you think?

tested it on vue playground:
https://play.vuejs.org/#eNqFUttu00AQ/ZVhX1KkYAvCU+VEAlQJkLgoIHhgEXLtiet2PbvaS5rK+N+ZXSfGRUDfdubM2Tkzc3rxwphsH1Cci8JVtjUeHPpgQJXUrKXwToqNpLYz2nroweIOBthZ3cGCaYsJeqU7M+alyPIYxW+lkCSp0uQ8dK6BdfzgbPEaldLwVVtVP1o8llTkY2/uxIHHzqjSI0cAxdXTTd8n8jAUOUcpm9rtn3S6RsUyGZYCcsaKfKKLJcvn3ru2ya6dJp6xj2QpKma3Cu0H41vWJsU5JCRiJUu7fZty3gZcnvLVFVY3f8lfu0PMSfHRokO755knzJe2QT/CF5/e44HfE8jag+Lq/4BbdFqFqHEsexmoZtmzuqT2TTpBS81nd3HwSO40VBQaK4dULwVfJC7uX6P/lrvKVoknaeAtnq75gEnwkJzg7wzCtqxb/aVUAWEdf/oJl1orLPm+MXDestzxvcWy1qTuikA3pG/p2/d04hmwxUrbuhhJSzjWbY5lFLpL3snxrdTMcdEdRxFQ464lfBdTxUzdyNmc3XfhHx5syYRxsDhtutTcfFObtIb7DvyxRxvvwbtbZc+zZysx/AJSRitQ

with the original it drops Object and null from the type:
https://play.vuejs.org/#eNqFUttuEzEQ/ZXBLylS2BWEp2oTCVAlQOKigOABI7TdnWzdem3LlzTVdv+dsZ0sKerlzXPOjOfMzBnYG2OKbUB2yirXWGE8OPTBgKxVt+TMO85WXIneaOthAIsbGGFjdQ8zKptN1Dvdm4xzVpQxit9yxhVXjVbOQ+86WMYPTmbvUUoNP7WV7bPZc66qMvemThR47I2sPVIEUF28XA1DKh7HqqQooand9kWvW5Qkk2jOoCSuKqdyNif51HsjuuLSaUUzDrGYs4aqhUT7xXhB2jg7hcREriZp1x8T5m3A+QFvLrC5uge/dLuIcfbVokO7pZknzte2Q5/ps2+fcUfviSTtQVL2I+QanZYhasxpb4NqSfZRXlL7IZ1AqO67O9t5VO4wVBQaM8eUzxldJC7uodH/yV0Ui1TH1UhbPFzzCZPgLjnB3xiEdd0K/aOWAWEZf7qFc60l1nTfGDhvSW5+W6xbreQNBHWl9LX69Tvj6z1erbHRtq1yzfyQtkpGuAUV+nNayf4t5ZHhojn2GqDFjVD4KULVkbhcszq5a8L/LCiUCXmuOGw61LH3pjZpC3cN+GeLNp6DVrcoXhevFmz8C0O8KxY=

@xvaara xvaara reopened this Apr 17, 2024
@VividLemon
Copy link
Member

I've tracked this down to the line in RadioTypes.ts:
| readonly unknown[] should be | Readonly<unknown[]> I think?

@VividLemon what do you think?

tested it on vue playground:
https://play.vuejs.org/#eNqFUttu00AQ/ZVhX1KkYAvCU+VEAlQJkLgoIHhgEXLtiet2PbvaS5rK+N+ZXSfGRUDfdubM2Tkzc3rxwphsH1Cci8JVtjUeHPpgQJXUrKXwToqNpLYz2nroweIOBthZ3cGCaYsJeqU7M+alyPIYxW+lkCSp0uQ8dK6BdfzgbPEaldLwVVtVP1o8llTkY2/uxIHHzqjSI0cAxdXTTd8n8jAUOUcpm9rtn3S6RsUyGZYCcsaKfKKLJcvn3ru2ya6dJp6xj2QpKma3Cu0H41vWJsU5JCRiJUu7fZty3gZcnvLVFVY3f8lfu0PMSfHRokO755knzJe2QT/CF5/e44HfE8jag+Lq/4BbdFqFqHEsexmoZtmzuqT2TTpBS81nd3HwSO40VBQaK4dULwVfJC7uX6P/lrvKVoknaeAtnq75gEnwkJzg7wzCtqxb/aVUAWEdf/oJl1orLPm+MXDestzxvcWy1qTuikA3pG/p2/d04hmwxUrbuhhJSzjWbY5lFLpL3snxrdTMcdEdRxFQ464lfBdTxUzdyNmc3XfhHx5syYRxsDhtutTcfFObtIb7DvyxRxvvwbtbZc+zZysx/AJSRitQ

with the original it drops Object and null from the type:
https://play.vuejs.org/#eNqFUttuEzEQ/ZXBLylS2BWEp2oTCVAlQOKigOABI7TdnWzdem3LlzTVdv+dsZ0sKerlzXPOjOfMzBnYG2OKbUB2yirXWGE8OPTBgKxVt+TMO85WXIneaOthAIsbGGFjdQ8zKptN1Dvdm4xzVpQxit9yxhVXjVbOQ+86WMYPTmbvUUoNP7WV7bPZc66qMvemThR47I2sPVIEUF28XA1DKh7HqqQooand9kWvW5Qkk2jOoCSuKqdyNif51HsjuuLSaUUzDrGYs4aqhUT7xXhB2jg7hcREriZp1x8T5m3A+QFvLrC5uge/dLuIcfbVokO7pZknzte2Q5/ps2+fcUfviSTtQVL2I+QanZYhasxpb4NqSfZRXlL7IZ1AqO67O9t5VO4wVBQaM8eUzxldJC7uodH/yV0Ui1TH1UhbPFzzCZPgLjnB3xiEdd0K/aOWAWEZf7qFc60l1nTfGDhvSW5+W6xbreQNBHWl9LX69Tvj6z1erbHRtq1yzfyQtkpGuAUV+nNayf4t5ZHhojn2GqDFjVD4KULVkbhcszq5a8L/LCiUCXmuOGw61LH3pjZpC3cN+GeLNp6DVrcoXhevFmz8C0O8KxY=

This would be a vue bug if true. readonly number[] is the same as ReadonlyArray

@VividLemon
Copy link
Member

vuejs/core#10726

@dwgray
Copy link
Contributor

dwgray commented Apr 18, 2024

I tried applying theses changes to the two affected classes, and while they working in the vue.js playground they didn't seem to work in the actual code. I have a draft PR with details: #1856

@xvaara
Copy link
Contributor

xvaara commented Apr 18, 2024

@VividLemon do we wait for vue to fix this or? they labeled it p3-minor-bug so I'm not holding my breath for a fix any time soon.

@VividLemon
Copy link
Member

Likely easier to fix it in Vue than here

@suzumejakku
Copy link
Author

suzumejakku commented Apr 19, 2024

Thanks for 0.17.1, it solved the issue when the value of the BFormRadioGroup is a String. I got the following error when the value is an Integer though:

[Vue warn]: Invalid prop: type check failed for prop "modelValue". Expected Boolean | String, got Number with value 2.

Is this related to the issue you opened in vuejs/core ?

@xvaara
Copy link
Contributor

xvaara commented Apr 19, 2024

Thanks for 0.17.1, it solved the issue when the value of the BFormRadioGroup is a String. I got the following error when the value is an Integer though:

[Vue warn]: Invalid prop: type check failed for prop "modelValue". Expected Boolean | String, got Number with value 2.

Is this related to the issue you opened in vuejs/core ?

Yes, it's the same bug.

@dwgray
Copy link
Contributor

dwgray commented Apr 22, 2024

@VividLemon & @xvaara - How would you feel about reverting the use of defineModel to useVModel in the BForm* classes, along the lines of what I suggested in #1847 ?

My reasoning:

  • Based on the exploration I did in fix(BForm*): Workaround for sfc type inference issues in BFormRadio*and BFormChecbox* #1856, my assessment is that defineModel isn't complete enough to handle the cases we're throwing at it - this extends beyond the specific bug that @VividLemon currently has filed against vuejs core. I also was unable to find workaround to 'fool' defineModel into infering the correct types in all cases.
  • vuejs/core/#10726 is marked as p3-minor.
  • I don't think we'll get a fix for both this and any other surrounding issues any time soon
  • Even if one of us is willing to jump into vusjs/core to fix these issue, we have no control over their release cycle.
  • I'm not sure what we're getting from defineModel other than moving towards the more core supported way of doing this, which I am completely behind, once they actually support the cases we need
  • Is there something else that we're getting out of the external typing of the vue components? I feel like I may be missing something here?

Assuming my reasoning is sound, let me know what you'd like different from what I currently have in #1847 and I'll be happy to iterate on that until I get it through. Otherwise, I'm interested in any other thoughts on how to get past these warnings.

@VividLemon
Copy link
Member

It's not an issue with defineModel. It's also a development warning that I'm not too concerned about. Regardless, I'll look into seeing if I can get someone to review it

@dwgray
Copy link
Contributor

dwgray commented Apr 22, 2024

It's not an issue with defineModel. It's also a development warning that I'm not too concerned about. Regardless, I'll look into seeing if I can get someone to review it

Thanks! I hadn't noticed that there is a candidate fix already. If I want to test this against the cases I'm interested in, do you happen to know how I would do that? Do I have to do a private build of vue? Or is there a way to monkey-patch the sfc compiler?

@xvaara
Copy link
Contributor

xvaara commented Apr 23, 2024

It seems to be fixed for dev build on 3.4.24, but production build is still broken.

@VividLemon VividLemon changed the title modelValue prop of BFormRadioGroup warns of type check failure Vue core issue tracker Vue warns of a type check failure Apr 24, 2024
@VividLemon
Copy link
Member

This issue doesn't really concern bvn. It's a Vue issue. I'm only keeping it around because it affects our lib

@VividLemon
Copy link
Member

#1864 seems related. Although not directly.

@bootstrap-vue-next bootstrap-vue-next locked and limited conversation to collaborators Apr 24, 2024
@xvaara
Copy link
Contributor

xvaara commented Apr 24, 2024

It's seems to be fixed in the latest release 3.4.25. I'll test tomorrow and release a new version if it fixes this for us.

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
4 participants