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

Repro engines + visit test issue #14085

Closed
wants to merge 2 commits into from

Conversation

asakusuma
Copy link
Contributor

@asakusuma asakusuma commented Aug 18, 2016

Repro #14086

assert.ok(instance instanceof ApplicationInstance, 'promise is resolved with an ApplicationInstance');
assert.strictEqual(jQuery('#qunit-fixture').children().length, 0, 'there are still no elements in the fixture element after visit');
assert.strictEqual(0, 0, 'there are still no elements in the fixture element after visit');
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem correct?

Copy link
Contributor Author

@asakusuma asakusuma Aug 18, 2016

Choose a reason for hiding this comment

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

@chancancode I'm just reproducing a problem, not actually expecting this to get merged. This set of changes should not fail the test suite, but it does.

Copy link
Member

Choose a reason for hiding this comment

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

I see 👀

@rwjblue
Copy link
Member

rwjblue commented Aug 20, 2016

I chatted with @krisselden and @dgeb earlier today about this, and we think we may have come up with an explanation here. Basically, the renderer is not being shared between the host and the engine. This ends up causing both app renderer and engine renderer to setup the normal run looping things, and causes it to end up being out of sync.

Right now this is the list of instances shared between the host and the engine. We think that the renderer and environment need to be added to that list.

@homu
Copy link
Contributor

homu commented Aug 27, 2016

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

@rwjblue
Copy link
Member

rwjblue commented Aug 27, 2016

I believe this issue is resolved. Closing for now, but I can reopen if you see it crop up again.

@rwjblue rwjblue closed this Aug 27, 2016
duggiefresh added a commit to duggiefresh/ember.js that referenced this pull request Aug 29, 2016
toddjordan pushed a commit to toddjordan/ember.js that referenced this pull request Sep 9, 2016
webark pushed a commit to webark/ember.js that referenced this pull request Oct 6, 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.

4 participants