[Glimmer2] Remove {{#view}} helper tests
#13235
Conversation
|
Going to follow up with another PR to add some tests to |
|
|
||
| QUnit.test('should teardown observers from bound properties on rerender', function() { |
There was a problem hiding this comment.
chadhietala
changed the title
{{#view}} helper{{#view}} helper
| QUnit.test('Child views created using the view helper should have their parent view set properly', function() { | ||
| var template = '{{#view}}{{#view}}{{view}}{{/view}}{{/view}}'; | ||
|
|
||
| QUnit.test('Should not apply classes when bound property specified is false', function() { |
There was a problem hiding this comment.
|
Will delete all the tests once I get a thumbs up. |
| QUnit.test('Child views created using the view helper should have their IDs registered for events', function() { | ||
| var template = '{{view}}{{view id="templateViewTest"}}'; | ||
|
|
||
| QUnit.test('Should apply classes of the dasherized property name when bound property specified is true', function() { |
There was a problem hiding this comment.
Appears to be {{#view}} specific.
|
Merged #13236, does this need to be updated? |
| @@ -35,7 +34,6 @@ registerPlugin('ast', AssertNoEachIn); | |||
|
|
|||
| if (!_Ember.ENV._ENABLE_LEGACY_VIEW_SUPPORT) { | |||
| registerPlugin('ast', AssertNoViewAndControllerPaths); | |||
| registerPlugin('ast', AssertNoViewHelper); | |||
There was a problem hiding this comment.
This likely should stick around (moved outside the if) to provide feedback to folks that might try to use {{view 'foo'}} after {{view keyword was removed.
|
@rwjblue Not sure how people feel about deleting the tests but keep the helper around. I was thinking we would wait till the |
|
There are pending PRs that remove some of the |
|
@chadhietala - I was suggesting that we want to keep the transform that asserts when using |
chadhietala
force-pushed
the
remove-view-helper-tests
branch
from
fd6a734
to
3bf99c1
Compare
|
@rwjblue Yea that makes sense. What your thoughts about the coverage? Is it ok if we just delete the tests and not the |
|
The deletion definitely needs to happen, but I'd rather keep queue up the deletions and rebase (or merge master in here) as we go to land it all at once. |
|
☔ The latest upstream changes (presumably #13192) made this pull request unmergeable. Please resolve the merge conflicts. |
chadhietala
force-pushed
the
remove-view-helper-tests
branch
from
3bf99c1
to
bfc1bf1
Compare
chadhietala
force-pushed
the
remove-view-helper-tests
branch
from
bfc1bf1
to
9612499
Compare
chadhietala
changed the title
{{#view}} helper{{#view}} helper tests
|
So lets land the test removals and then I'll open another PR that removes the helper, but it will needed to be shepherded until all the |
|
☔ The latest upstream changes (presumably #13319) made this pull request unmergeable. Please resolve the merge conflicts. |
|
These files have been deleted, closing... |
This currently removes just the tests related to the
viewhelper. I have left some tests in the file because I have a feeling that these tests are cross cutting and were used as coverage for components. I could be 100% incorrect about this but would like to hear from @rwjblue and @chancancode before they are removed 💣.Furthermore, it looks like we will need to dereference all the
viewhelpers in all of the tests before we can finally remove the helper. Luckily majority of the tests in #13127 should be addressing this.Part of #13127