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.

Sign up for GitHub

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

Jump to bottom

Remove dirty checks for objects #3596

Closed
wants to merge 30 commits into from
Closed

Remove dirty checks for objects #3596

wants to merge 30 commits into from

Conversation

kaste
Copy link
Contributor

@kaste kaste commented Apr 17, 2016 •
edited
Loading

To come to an end here. Assembles the following PRs:

Then we can just remove the dirty check for object values 6d24b87.

Related #3343.

kevinpschaaf and others added 30 commits April 10, 2016 15:40
@kevinpschaaf @kaste
Eliminate creating accessors for parent props; use Bind API directly.
59b5f24
@kaste
Do not pre-store effects in the accessors.
35a533a
@kaste
Add tests
3537bd9
@kaste
code path 1 (one-to-one binding)
30ddcc6 
Note: `this[property] = value` implicitly guards against setting read
onlys. This check is now explicit.
@kaste
code path 2 (deep binding)
9591b25 
Note: `set`'s third argument is unused within the library and should be
removed.

Note: No readOnly check here, why?

Note: Additional dirty check in accessors.html redundant.

Obvious code duplication between annotation effect and annotation path-
effect. Merge both?
@kaste
code path 3 (path notifications)
7c361b9
@kaste
Remove superfluous dirty check.
af97faf
@kaste
Pass fromAbove et.al when setting properties (not paths) via set.
e9067bd 
Also opens `set` to set read-only properties. Note: computed properties
could be set as well. Nobody should do this.
@kaste
Remove third argument 'root' from set()
e9e1ebb 
That's harsh, but

  1. set() is a public method, so +1 for clean interface
  2. totally not used within the framework

Actually I think it was never used. For the record: could have been used
to set a path without notifying. (There is an equivalent arg for `_get` which
is used though during config.)
@kaste
Inline isStructured variable for the reading pleasure.
4182ab8
@kaste
Refactor _applyEffectValue.
82cd08a
@kaste
Expose bare minimum to add effects dynamically.
523ef03
@kaste
Use functionPath-effects in templatizer.
f6b107d 
Removes _pathEffector override completely.
Replaces _notifyPathUp override.
@kaste
Merge branch 'bare-minimum-for-custom-effects2' into link-path-2
69f384c
@kaste
Implement linkPath as property effect.
644e290 
Avoid dirty checking values, instead use a lock.
@kaste
Ensure effects run exactly once during config.
bdcb498 
This reduces the addiction to dirty check values.
@kaste
Make _distributeConfig straightforward.
dd776fd
@kaste
Remove unused propertyName from annotation notes.
4966354
@kaste
Ensure fromAbove in _forwardParentProp.
072dcff 
The general rule here is: `_forwardParentProp/Path` must assume fromAbove
is true; for `_forwardInstanceProp/Path` assume fromAbove is false.
@kaste
Ensure fromAbove for downwards bindings.
b024646
@kaste
Assume attribute changes as coming from above.
cad8224
@kaste
Make _notifyPath slightly more readable and efficient.
dc60ebb
@kaste
Merge branch 'ensure-from-above-for-templatizer' into no-dirty-checks…
d6c8856 
…-for-objects
@kaste
Merge branch 'ensure-from-above-for-downwards-bindings' into no-dirty…
24ce454 
…-checks-for-objects
@kaste
Merge branch 'assume-from-above-when-attributes-change' into no-dirty…
77a6c7b 
…-checks-for-objects
@kaste
Merge branch 'upwards-does-not-echo-downwards' into no-dirty-checks-f…
38cf8ef 
…or-objects

# Conflicts:
#	src/standard/notify-path.html
@kaste
Merge branch 'run-each-effect-exactly-once-during-config' into no-dir…
96fbe80 
…ty-checks-for-objects

# Conflicts:
#	src/lib/bind/accessors.html
@kaste
Merge branch 'bare-minimum-for-custom-effects2' into no-dirty-checks-…
a295edc 
…for-objects

# Conflicts:
#	src/lib/bind/accessors.html
#	src/standard/notify-path.html
#	test/unit/bind-elements.html
#	test/unit/bind.html
@kaste
Merge branch 'link-path-as-effect' into no-dirty-checks-for-objects
8cfb586
@kaste
Assume objects are always dirty.
6d24b87
@kaste
Copy link
Contributor Author

kaste commented Apr 17, 2016

I think we should have these API changes/additions:

  • set(path, value, fromAbove?)
  • get(path)
  • notifyUpwards(path, value?) or propagateUpwards
  • assumeChanged(path, fromAbove?)

whereby path can be simple or deep (foo, foo.bar)

@kaste kaste mentioned this pull request Apr 17, 2016
Closed
@kaste kaste mentioned this pull request Apr 21, 2016
Closed
@tjsavage tjsavage added the 1.x label Sep 8, 2016
@devinivy
Copy link

@kaste I see that many of your ideas, this one included, made their way into the implementation of v2. Just wanted to say thanks!

@TimvdLippe
Copy link
Contributor

As @devinivy pointed out your suggestions have been adopted and are available in Polymer 2 as the MutableData mixin. Thanks!

@TimvdLippe TimvdLippe closed this Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
1.x cla: yes needs discussion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kaste @devinivy @TimvdLippe @dfreedm @tjsavage @googlebot @kevinpschaaf