Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: convert emits to props, fix #13104 #13105

Closed
wants to merge 2 commits into from

Conversation

so1ve
Copy link
Member

@so1ve so1ve commented Oct 23, 2023

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Converts emits to props.

Other information:

@yyx990803
Copy link
Member

@so1ve I tested this locally and it doesn't seem to fix #13104 - is this something you still intend to work on? We might be able to get this in the final patch release before EOL.

@so1ve
Copy link
Member Author

so1ve commented Dec 6, 2023

I'll take a look and add tests

@so1ve
Copy link
Member Author

so1ve commented Dec 6, 2023

I need some help, I couldn't get emits from extends and mixin work...

@so1ve so1ve closed this Dec 6, 2023
@so1ve
Copy link
Member Author

so1ve commented Dec 6, 2023

This fix is beyond my capabilities and I hope someone else can fix it.

@zcf0508
Copy link

zcf0508 commented Dec 15, 2023

@so1ve I tested this locally and it doesn't seem to fix #13104 - is this something you still intend to work on? We might be able to get this in the final patch release before EOL.

#13133

https://github.com/vuejs/vue/pull/13133/files#diff-d26123bca327fbaf4e0ce33d7085fcaf9f4a39be4f76a1d0c15e0843afb1af9fR153

Maybe this is the key line to resolve the error of #13104

@so1ve
Copy link
Member Author

so1ve commented Dec 15, 2023

No, your implementation doesn't take extends and mixin into account

@zcf0508
Copy link

zcf0508 commented Dec 18, 2023

No, your implementation doesn't take extends and mixin into account

Yes, you are right, I'm working on it. But now I have some questions. I see you add a test that you can get emit functions fromprops in setup function, but the props does not expose those functions. It is a right type test?

image

image image

@so1ve
Copy link
Member Author

so1ve commented Dec 18, 2023

Yes, because it can be undefined

@zcf0508
Copy link

zcf0508 commented Dec 18, 2023

Does it mean that If a component use props.onFoo in runtime, the function will never work? Is the onFoo only used for type inference?

@so1ve
Copy link
Member Author

so1ve commented Dec 18, 2023

Yes, seemed to be true

@zcf0508
Copy link

zcf0508 commented Dec 18, 2023

image

#13133

I think it works with extends and mixin now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vue 2.7 strictTemplates error
3 participants