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

Override component watcher with mounting options #1392

Merged
merged 5 commits into from
Jan 6, 2020

Conversation

lmiller1990
Copy link
Member

@lmiller1990 lmiller1990 commented Jan 6, 2020

resolves #1391 . We should override a component's watcher if one is provided in the mounting options. The issue explains the bug a bit better and provides an example.

@lmiller1990 lmiller1990 changed the title Wip: Override component watcher with mounting options Override component watcher with mounting options Jan 6, 2020
Copy link
Collaborator

@JessicaSachs JessicaSachs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there might be alternate data structures for watch... Based on the examples here, we need to support: Strings, Functions, Arrays, Objects.

Does includes work as desired with all of those data types?

@dobromir-hristov
Copy link
Contributor

Jess, the key is always a string :)

Copy link
Member

@afontcu afontcu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Quick question: what would happen if the instance watcher is immediately invoked but the component one isn't (or the other way around)? Would this logic override the value from the instance watcher no matter what?

@dobromir-hristov
Copy link
Contributor

The one you pass to the mount should always override the entire watcher definition, imho. Its up to you, as a dev to take care if its immediate, deep or whatever, no?

@afontcu
Copy link
Member

afontcu commented Jan 6, 2020

The one you pass to the mount should always override the entire watcher definition, imho. Its up to you, as a dev to take care if its immediate, deep or whatever, no?

Sure! I guess what I wanted to confirm is that the current implementation overrides the entire definion, not only the handler :) I'd assume so, but wanted to make sure 👍

@JessicaSachs JessicaSachs merged commit ee09270 into dev Jan 6, 2020
@lmiller1990 lmiller1990 deleted the issue-1391-watchers branch January 6, 2020 20:56
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.

watch mounting option should overwrite option on same component
4 participants