Skip to content

Decouple ember link_to_test from globals resolver#15319

Merged
mixonic merged 1 commit intoemberjs:masterfrom
mixonic:link_to_test
Jun 5, 2017
Merged

Decouple ember link_to_test from globals resolver#15319
mixonic merged 1 commit intoemberjs:masterfrom
mixonic:link_to_test

Conversation

@mixonic
Copy link
Member

@mixonic mixonic commented Jun 2, 2017

References #15058

It is a big one. About 30% of the way through converting the file, I'd like to finish the conversion in a single commit.

import { jQuery } from 'ember-views';
import { compile } from 'ember-template-compiler';
import { setTemplates, setTemplate } from 'ember-glimmer';
import { EMBER_IMPROVED_INSTRUMENTATION } from 'ember-debug';
Copy link
Member Author

Choose a reason for hiding this comment

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

Pretty sure this is just wrong, and the tests in this file controlled by this value were not running.

return run(applicationInstance, 'visit', url, options);
} else {
return run(this.application, 'visit', url, options).then(instance => {
this.applicationInstance = instance;
Copy link
Member Author

@mixonic mixonic Jun 2, 2017

Choose a reason for hiding this comment

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

b/c this .then is not inside the runloop, this.applicationInstance was being assigned after this.visit('/foo'). The more predictable behavior is to wrap the then in a runloop so we can presume the instance is set on the next line of code after this.visit

@mixonic mixonic force-pushed the link_to_test branch 3 times, most recently from c0a4379 to 7dfb4e1 Compare June 5, 2017 22:12
@mixonic mixonic changed the title WIP: Decouple ember link_to_test from globals resolver Decouple ember link_to_test from globals resolver Jun 5, 2017
@mixonic mixonic merged commit 1c720c8 into emberjs:master Jun 5, 2017
@mixonic mixonic deleted the link_to_test branch June 5, 2017 23:38
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.

1 participant