[BUGFIX release] this.$() undefined in willDestroyElement hook #14685
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krisselden
requested changes
Dec 7, 2016
| @@ -9,6 +10,12 @@ import _default from './default'; | |||
| const destroying = Object.create(_default); | |||
|
|
|||
| assign(destroying, { | |||
| $(view, sel) { | |||
There was a problem hiding this comment.
lets just move it out of states and just make this the method
There was a problem hiding this comment.
@krisselden Thanks for review! View mixin seems to be the most appropriate place for that method. PR updated.
|
@canufeel Thank you! |
|
With the context of #14666, thoughts on if this be worthy of cherry-picking back to 2.10.x since it introduced unexpected behavior during an upgrade from 2.9 to 2.10? Otherwise I think we might end up waiting to upgrade from 2.9 until 2.11 stable is cut (which is no problem). |
|
Yes I'll do the cherry pick and release it next week! |
|
Ok, awesome! 🎉 |
|
@chancancode this is hitting our own app 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trying to access
this.$()inwillDestroyElementnow returnsundefinedaswillDestroyElementhook is run after the component enters thedestroyingstate since 3dcde0f was merged (2.9.0-beta.1). In versions before thatwillDestroyElementwas called before transitioning todestroyingstate so the component was usinginDomstate object that would processthis.$()properly.Technically the element is still accessible but we have to use
$(this.element)instead ofthis.$()which makes upgrading cumbersome as it is breaking apps that utilisewillDestroyElementto deregister eventHandlers previously registered on component element.Fixes #14666