Skip to content

Commit

Permalink
Do not register a router service for Ember <1.13
Browse files Browse the repository at this point in the history
Versions of Ember prior to 1.13 require a full router booted by the app,
or no router at all. This ensures they receive no router at all instead
of the 1.13-safe one.
  • Loading branch information
mixonic committed Sep 10, 2015
1 parent 5f148ff commit 7df5b66
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 5 deletions.
8 changes: 8 additions & 0 deletions lib/ember-test-helpers/has-ember-version.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Ember from 'ember';

export default function hasEmberVersion(major, minor) {
var numbers = Ember.VERSION.split('-')[0].split('.');
var actualMajor = parseInt(numbers[0], 10);
var actualMinor = parseInt(numbers[1], 10);
return actualMajor > major || (actualMajor === major && actualMinor >= minor);
}
11 changes: 7 additions & 4 deletions lib/ember-test-helpers/test-module.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getContext, setContext, unsetContext } from './test-context';
import { Klass } from 'klassy';
import { getResolver } from './test-resolver';
import buildRegistry from './build-registry';
import hasEmberVersion from './has-ember-version';

export default Klass.extend({
init: function(subjectName, description, callbacks) {
Expand Down Expand Up @@ -234,10 +235,12 @@ export default Klass.extend({
this.container = items.container;
this.registry = items.registry;

var thingToRegisterWith = this.registry || this.container;
var router = resolver.resolve('router:main');
router = router || Ember.Router.extend();
thingToRegisterWith.register('router:main', router);
if (hasEmberVersion(1, 13)) {
var thingToRegisterWith = this.registry || this.container;
var router = resolver.resolve('router:main');
router = router || Ember.Router.extend();
thingToRegisterWith.register('router:main', router);
}
},

_setupIsolatedContainer: function() {
Expand Down
3 changes: 2 additions & 1 deletion tests/test-module-for-component-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Ember from 'ember';
import { TestModuleForComponent } from 'ember-test-helpers';
import hasEmberVersion from 'ember-test-helpers/has-ember-version';
import test from 'tests/test-support/qunit-test';
import qunitModuleFor from 'tests/test-support/qunit-module-for';
import { setResolverRegistry } from 'tests/test-support/resolver';
Expand Down Expand Up @@ -237,7 +238,7 @@ moduleForComponent('changing-color', 'component:changing-color -- handles closur
integration: true
});

if (!/^1\.(11|12)/.test(Ember.VERSION)) {
if (hasEmberVersion(1,13)) {

This comment has been minimized.

Copy link
@MiguelMadero

MiguelMadero Apr 12, 2016

@mixonic looks like this change broke controller and route tests in 1.11 and 1.12 since they have an injection for router:main. Also, on these versions, resolve resolver.resolve('router:main'); already returns the full router, not the 1.13 safe router.

This comment has been minimized.

Copy link
@mixonic

mixonic Apr 13, 2016

Author Member

0_o seems like a good thing to open an issue for. crosslink to the PR here: #100

How would this specific line be in play?

This is from nearly 7 months ago so I'm a bit fuzzy on what the exact issues were.

Are you seeing this failure? emberjs/ember.js#11825 (comment)

This comment has been minimized.

Copy link
@MiguelMadero

MiguelMadero Apr 13, 2016

Done, thanks #151

test('handles a closure actions', function() {
expect(1);
this.on('colorChange', function(arg) { equal(arg, 'foo'); });
Expand Down
5 changes: 5 additions & 0 deletions tests/test-module-for-integration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ test('it can render a template', function() {
equal(this.$('span').text(), 'Hello');
});

test('it can render a link-to', function() {
this.render("{{link-to 'Hi' 'index'}}");
ok(true, 'it renders without fail');
});

test('it complains if you try to use bare render', function() {
var self = this;
throws(function() {
Expand Down

0 comments on commit 7df5b66

Please sign in to comment.