-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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(runtime-core): optimization the logic of setting __props
(close #2651)
#2654
Conversation
f05d130
to
20da7a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a test?
__props
(close #2651)
@@ -169,7 +169,11 @@ export function normalizeEmitsOptions( | |||
extend(normalized, normalizeEmitsOptions(raw, appContext, true)) | |||
} | |||
if (!asMixin && appContext.mixins.length) { | |||
appContext.mixins.forEach(extendEmits) | |||
appContext.mixins.forEach(raw => { | |||
if (Object.keys(raw).length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think it makes more sense to add this check at componentEmits.ts
where the warning is emitted. Or is there any reason not to adapt the warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think it makes more sense to add this check at
componentEmits.ts
where the warning is emitted. Or is there any reason not to adapt the warning?
if (__DEV__) {
const {
emitsOptions,
propsOptions: [propsOptions]
} = instance
if (emitsOptions) {
if (!(event in emitsOptions)) {
if (!propsOptions || !(toHandlerKey(event) in propsOptions)) {
warn(
`Component emitted event "${event}" but it is neither declared in ` +
`the emits option nor as an "${toHandlerKey(event)}" prop.`
)
}
}
The normal logic is: if emitsOptions
is null
there is no warning.
if app.mixin({})
or mixin some object without emits
property, the emitsOptions
should be null.
I found emitsOptions
is {}
when I was debugging. Because in normalizeEmitsOptions
let normalized: ObjectEmitsOptions = {}
//...
const extendEmits = (raw: ComponentOptions) => {
hasExtends = true
extend(normalized, normalizeEmitsOptions(raw, appContext, true))
}
if (!asMixin && appContext.mixins.length) {
// appContext.mixins is [{__props:[]}], it should be [{}] because we mixin `{}`
appContext.mixins.forEach(extendEmits)
}
The value of app.mixins
becomes [{__props:[]}]
after normalizePropsOptions
called.
It should be [{}]
. in normalizePropsOptions
//...
if (!raw && !hasExtends) {
return (comp.__props = EMPTY_ARR as any)
}
//...
I think comp.__props
should not be defined if !raw && !hasExtends
So I did not modify the function emit()
where the warning is emitted.
Instead, normalizePropsOptions
and normalizeEmitsOptions
are modified
Did I express it clearly enough ? Please correct me if my understanding is wrong.
@posva can you take another look here? |
See better fix in 60d777d |
close #2651
if
app.mixin({})
called, app.mixins will be[{}]
.It will become
[{__props:[]}]
afternormalizePropsOptions
called.