Unobserve attributes #3
Conversation
|
@semmel thank you very much for the PR. Sorry for the delay, for some reason I am not receiving notifications for this repo. Also regarding roll-up, in theory running |
| @@ -34,7 +34,7 @@ This is invoked when an attribute from the `observedAttributes` list is added, c | |||
| removed. This could be useful to make network request for updating the data. | |||
|
|
|||
| ```javascript | |||
| onAttributeChanged(element, ({ name, current }) => { | |||
| let unsubscribe = onAttributeChanged(element, ({ name, current }) => { | |||
There was a problem hiding this comment.
Can we use const instead of let here? Since is not going to change it make more sense a const.
| element = document.querySelector('attribute-test-element'); | ||
|
|
||
| onAttributeChanged(element, ({name, current, old}) => { | ||
| if (name !== "foo") { |
There was a problem hiding this comment.
We could use a mock function and check if that function was called or not called in the other test. In that way the test will be simpler and we don't need to throw errors
There was a problem hiding this comment.
Sorry, I don't get it 🤔. Could you describe it more?
I did that, but I could not find any differences :-/ |
For wrapping attribute changes in my reactive streams I needed a way to unsubscribe the callback again.
This PR also adds a few tests around
onAttributeChanged.(I did not quite understand the need for the additional
jesttests, why is karma not sufficient?)Hope all this is non-breaking since I use the return value of
onAttributeChangedwhich is up to now not documented.I did not build the doc files, just edited the sources.
I needed to bump karma-rollup-preprocessor because the old is not compatible with my more recent rollup.