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

Make ComponentNodeManager.rerender use RenderEnv #11341

Merged
merged 1 commit into from
Jun 4, 2015

Conversation

jmurphyau
Copy link
Contributor

Fixes #11340.

ComponentNodeManager.render()

let env = _env.childWithView(component);

ComponentNodeManager.rerender()

let env = _env;
env = assign({ view: component }, env);

This causes issues in later steps where something calls childWithView on env - which no longer exists.

@jmurphyau
Copy link
Contributor Author

/cc @tomdale

Fixes emberjs#11340.

Test 'non-block with each rendering child components' in 'component_invocation_test.js' demonstrates a 'real world' test that is failing.
@jmurphyau
Copy link
Contributor Author

Wasn't sure where to put the test - 'component_invocation_test.js' seems most appropriate but it looks like its getting a bit crowded..

rwjblue added a commit that referenced this pull request Jun 4, 2015
Make ComponentNodeManager.rerender use RenderEnv
@rwjblue rwjblue merged commit bdfd369 into emberjs:master Jun 4, 2015
@tomdale
Copy link
Member

tomdale commented Jun 4, 2015

Awesome job! Not sure how we missed this… @rwjblue, can we make sure this gets into beta with the other commits? ❤️ ❤️ ❤️

@rwjblue
Copy link
Member

rwjblue commented Jun 4, 2015

Yep, pulled into beta.

mixonic added a commit to mixonic/ember.js that referenced this pull request Apr 22, 2016
This test was added to assert Glimmer 1 passed around the render env
correctly. That `env` is present is an implementation detail and not
public API. Since the bug motivating these assertions is tested for
elsewhere, these Glimmer 1 specific tests can be removed.

See also:

* emberjs#11341
* emberjs@dfe32bc
toddjordan pushed a commit to toddjordan/ember.js that referenced this pull request Sep 9, 2016
This test was added to assert Glimmer 1 passed around the render env
correctly. That `env` is present is an implementation detail and not
public API. Since the bug motivating these assertions is tested for
elsewhere, these Glimmer 1 specific tests can be removed.

See also:

* emberjs#11341
* emberjs@dfe32bc
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.

3 participants