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.

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

[Glimmer2] Migrate and Remove block params test #13189

Merged
merged 1 commit into from
Apr 5, 2016

Conversation

Joelkang
Copy link
Contributor

Remove should raise error if helper not available: custom-helpers-test.js:287 already tests this
Remove basic block params usage: syntax/with-test.js:21 already tests this

Keeping components can yield values: Even though helpers/hash-test.js already tests yielding hashes, I added a test to components/curly-components.js to test yielding non-hash properties

Part of #13127

cc @chancancode

@@ -292,4 +292,43 @@ moduleFor('Syntax test: Multiple {{#with as}} helpers', class extends RenderingT
this.assertText('Los Pivots');
}

['@test nested {{#with}} blocks should have access to root context']() {
this.render(`{{name}}{{#with committer1.name as |name|}}[{{name}}{{#with committer2.name as |name|}}[{{name}}]{{/with}}{{name}}]{{/with}}{{name}}{{#with committer2.name as |name|}}[{{name}}{{#with committer1.name as |name|}}[{{name}}]{{/with}}{{name}}]{{/with}}{{name}}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been using the strip helper to keep things readable. It removes all leading whitespace and newlines. For example, check out https://github.com/emberjs/ember.js/blob/master/packages/ember-glimmer/tests/integration/syntax/each-in-test.js#L110-L116.

@chadhietala
Copy link
Contributor

Thanks @Joelkang! Looks good to me.

@homu
Copy link
Contributor

homu commented Mar 27, 2016

☔ The latest upstream changes (presumably #13190) made this pull request unmergeable. Please resolve the merge conflicts.

@Joelkang
Copy link
Contributor Author

Rebased, squashed and added the an R in INUR for the infinite loop test

@chancancode
Copy link
Member

@Joelkang looks like we lost the removals from the rebase 😄

@Joelkang
Copy link
Contributor Author

herp derp. fixed

@wycats wycats merged commit 9301fbf into emberjs:master Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants