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

Wrong merging of props and emits definitions from mixins #7989

Closed
lehni opened this issue Mar 30, 2023 · 1 comment · Fixed by #8052
Closed

Wrong merging of props and emits definitions from mixins #7989

lehni opened this issue Mar 30, 2023 · 1 comment · Fixed by #8052
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working

Comments

@lehni
Copy link
Contributor

lehni commented Mar 30, 2023

Vue version

3.2.47

Link to minimal reproduction

https://sfc.vuejs.org/#eNq1VEuL2zAQ/ivCFHxxLLq9GRPo3ksPLb3Ue3CTcaKwloQkJynG/72jl6082G7p7iXRjD59M988PGafpSyPA2RVVhvo5XNrYN1wQuotO7oDHvcf1z8GIJ/IF3ZmnDwOO11TdIbrrxwIjcb3k1gMvFnsmnrKmiaB0NQbxaRxENZLoQyxhJ0SPclLimebX55cW8p4jefr6xA0IVhAFgZnB9tC1w7Phow+143A1xy40RUZLUdh4xSRbbIo/KnpnG5WZD7iqm9ledCCYxEdWxMudJMhmedvMszA2k22N0bqilLdbWxaB10KtaN4KtXADeuhBN2vfilx0qCQuMmKhIOi8whqpYBvQYF6ifMKesMbRE0oxbXWFvugUca9GkklMAT5mWOZ8ieXEvTM1su7CpKbk3B/ewXO7sSg8qckgO3E3wKEgiHXfEbrt4SKfDOK8V2oBomvKx/Ye32jLrPr2NGlo9k5ZvPYajdX13PfmIvBf1iPI/kgpGGC65K3PZBpwtl/iAgZDkv2Cd658EHA0ghOXoUc01fOdfPq7vagkLAfVsftIlmV8yJEyemuxK7PoGQMXlwXWwqsOyLz0A44Gxwz1GLjBF9v2WwLIi1W/3KNUEHYzv9RkO57MmevUYDI1ytA8D0FyyfmfdrwZmJ9ov/UsWIOeqt8+gPEpA3m

Steps to reproduce

Define props and emits definitions as arrays in mixins, and use multiply mixins in a component. The defintions get wrongly merged, because the internal mergeOptions() helper uses the wrong merge strategies for these fields: mergeObjectOptions which ends up treating arrays as objects.

Both actually have an empty // TODO behind them, I imagine it's about this problem, and got forgotten:

props: mergeObjectOptions, // TODO
emits: mergeObjectOptions, // TODO

What is expected?

The fields should be correctly merged. For props, if one of the encountered props is an object instead of an array, all definitions should be converted to object notation and merged.

props: { "one": {}, "two": { "default": "two" } }
emits: [ "one", "two", "three", "four", "five", "six" ]

What is actually happening?

The arrays are being treated as objects, and their indices become object keys and override each other:

props: { "0": "one", "two": { "default": "two" } }
emits: { "0": "five", "1": "six", "2": "three", "3": "four" }
@lehni lehni changed the title Various bugs when merging props and emits definitions from mixins Wrong merging of props and emits definitions from mixins Mar 30, 2023
@edison1105 edison1105 added 🐞 bug Something isn't working 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Mar 30, 2023
@lehni
Copy link
Contributor Author

lehni commented Apr 11, 2023

Thanks @yyx990803! Somewhat related, I'd like to highlight this gnarly bug here in the devtools:

https://github.com/vuejs/devtools/issues/2037

Would be great to get this merged and released soon.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants