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

feat: support .sync modifier #54

Merged
merged 1 commit into from
Feb 10, 2020
Merged

feat: support .sync modifier #54

merged 1 commit into from
Feb 10, 2020

Conversation

jefrydco
Copy link
Contributor

@jefrydco jefrydco commented Feb 9, 2020

Fix #15

@jefrydco jefrydco requested a review from koca February 9, 2020 14:58
@koca
Copy link
Owner

koca commented Feb 9, 2020

Hey, thanks for the PR.
But why do we need the sync modifier? If the default v-model events solves the problem than lets use value prop and input event instead of code prop and change event.
What do you think?

@jefrydco
Copy link
Contributor Author

jefrydco commented Feb 9, 2020

Yes, I agree with you. My consideration is if we change the code prop and change event, it would be a breaking change. Because they are the main communication between this package and the rest of the world.

And themodel options in your component declaration already solved the problem as well, isn't it?

@koca
Copy link
Owner

koca commented Feb 10, 2020

Yep you are right that would be a breaking change. I will merge this and we can release removed version with v1.0.0? Sounds good?

@jefrydco
Copy link
Contributor Author

I will merge this and we can release removed version with v1.0.0?

You mean that we can change thecode props and change event with value props and input event, then release them with v1.0.0? If it yes, sounds good to me.

@koca
Copy link
Owner

koca commented Feb 10, 2020

Actually I was thinking about two options:
1 - merge this and publish v0.4.0. (this evening)
2 - merge this and publish v0.4.0 and remove .sync on the v1.0.0 release (I'm planining to release next weekend).

I'll go with the second option. I'm merging this and releasing a new version. Next week v1.0.0 i will deprecate for the sake of proper v-model.

@koca koca merged commit a2d7175 into koca:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support v-model
2 participants