[breaking] Remove view.$() call in render() of module for component
#27
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
This prevents a possible error when a callback returns `false` and therefore it skips the cache.
This is a breaking change. Previously, calling `this.render()` in a module-for-component would append the view and then return the view's `this.$()`. This changes `render` to only render the component and append it, returning `undefined`. Test users can still use `this.$()` to get the component's jquery-wrapped element. This avoids a possible issue in Ember >= 1.11.0-beta.5 where tagless components (`tagName = ''`) will give a deprecation warning when their `$()` method is called.
|
👍 - LGTM |
|
@bantic can we make We certain have projects that rely on |
|
@mixonic coming up... |
|
Updated
|
|
+1 |
|
@dgeb I think this is good to go, if you are happy with it |
dgeb
added a commit
that referenced
this pull request
Mar 17, 2015
[breaking] Remove `view.$()` call in `render()` of module for component
|
Good improvements - thanks @bantic! |
|
Just tagged v0.4.0. |
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.
from #26
This PR makes two changes:
cachedCallsobject internally in TestModule to track whether a given callback has been called. Previously, the code only did a check on the result (inthis.cache[key]), which means that a callback that returns a falsy value will (incorrectly, it seems) be called multiple times. This seemed likely to result in inconsistent behavior. (This change is necessary for the following change)this.callbacks.renderto not return anything. It simply renders the component. This is a breaking change. Previously,renderwould return the rendered view'sthis.$()method, but this is problematic because there are cases where a view's$()method should not be called. Specifically, in Ember 1.11.0.beta-5, callingthis.$()on a tagless component (wheretagName = '') will result in a failed assertion: "You cannot access this.$() on a component withtagName: \'\'specified.", making it difficult-to-impossible to test such a component in a unit test.Test users can still call
this.$()to get the jquery-wrapped element (in cases where that makes sense — if the component is tagless they will still get the failed assert mentioned above)cc @rwjblue @mixonic