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.

Sign up for GitHub

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

Jump to bottom

[DEPRECATION] Deprecate globals resolver #18436

Merged
merged 7 commits into from
Nov 19, 2019
Merged

[DEPRECATION] Deprecate globals resolver #18436

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f8c9a26
[DEPRECATION] Deprecate the globals resolver
Gaurav0 Sep 27, 2019
74194e9
fix tests
Gaurav0 Nov 18, 2019
2798cd2
fix test failure in prod
Gaurav0 Nov 18, 2019
86d59a0
Empty commit to try restarting build
Gaurav0 Nov 18, 2019
8709050
Address comments
Gaurav0 Nov 18, 2019
8b7d402
svelte condition
Gaurav0 Nov 18, 2019
e7b2c13
Correct version number.
Gaurav0 Nov 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { ApplicationTestCase, moduleFor, strip } from 'internal-test-helpers';
import {
ApplicationTestCase,
ModuleBasedTestResolver,
moduleFor,
strip,
} from 'internal-test-helpers';

import { ENV } from '@ember/-internals/environment';
import {
Expand Down Expand Up @@ -292,6 +297,7 @@ if (ENV._DEBUG_RENDER_TREE) {
'engine:foo',
class extends Engine {
isFooEngine = true;
Resolver = ModuleBasedTestResolver;

init() {
super.init(...arguments);
Expand Down Expand Up @@ -327,6 +333,8 @@ if (ENV._DEBUG_RENDER_TREE) {
this.add(
'engine:bar',
class extends Engine {
Resolver = ModuleBasedTestResolver;

init() {
super.init(...arguments);
this.register(
Expand Down Expand Up @@ -670,6 +678,7 @@ if (ENV._DEBUG_RENDER_TREE) {
'engine:foo',
class extends Engine {
isFooEngine = true;
Resolver = ModuleBasedTestResolver;

init() {
super.init(...arguments);
Expand Down
24 changes: 23 additions & 1 deletion packages/@ember/-internals/glimmer/tests/integration/application/engine-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { moduleFor, ApplicationTestCase, strip, runTaskNext } from 'internal-test-helpers';
import {
moduleFor,
ApplicationTestCase,
ModuleBasedTestResolver,
strip,
runTaskNext,
} from 'internal-test-helpers';

import { Component } from '@ember/-internals/glimmer';
import { Route } from '@ember/-internals/routing';
Expand Down Expand Up @@ -80,6 +86,8 @@ moduleFor(
this.add(
'engine:blog',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down Expand Up @@ -131,6 +139,8 @@ moduleFor(
this.add(
'engine:chat-engine',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register('template:application', compile('Engine'));
Expand Down Expand Up @@ -167,6 +177,8 @@ moduleFor(
this.add(
'engine:blog',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register('template:foo', compile('foo partial'));
Expand Down Expand Up @@ -202,6 +214,8 @@ moduleFor(
this.add(
'engine:chat-engine',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register('template:foo', compile('foo partial'));
Expand Down Expand Up @@ -230,6 +244,8 @@ moduleFor(
this.add(
'engine:chat-engine',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register('template:components/foo-bar', compile(`{{partial "troll"}}`));
Expand Down Expand Up @@ -289,6 +305,8 @@ moduleFor(
this.add(
'engine:blog',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);

Expand Down Expand Up @@ -344,6 +362,8 @@ moduleFor(
this.add(
'engine:blog',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down Expand Up @@ -486,6 +506,8 @@ moduleFor(
this.add(
'engine:blog',
Engine.extend({
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register('template:application', compile('Engine{{outlet}}'));
Expand Down
21 changes: 20 additions & 1 deletion packages/@ember/-internals/glimmer/tests/integration/mount-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { moduleFor, ApplicationTestCase, RenderingTestCase, runTask } from 'internal-test-helpers';
import {
moduleFor,
ApplicationTestCase,
ModuleBasedTestResolver,
RenderingTestCase,
runTask,
} from 'internal-test-helpers';

import { set } from '@ember/-internals/metal';
import { getOwner } from '@ember/-internals/owner';
Expand Down Expand Up @@ -48,6 +54,7 @@ moduleFor(
'engine:chat',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
Expand Down Expand Up @@ -165,6 +172,8 @@ moduleFor(
'engine:foo',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand All @@ -180,6 +189,8 @@ moduleFor(
'engine:bar',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down Expand Up @@ -244,6 +255,8 @@ moduleFor(
'engine:foo',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down Expand Up @@ -299,6 +312,8 @@ moduleFor(
'engine:paramEngine',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down Expand Up @@ -393,6 +408,8 @@ moduleFor(
'engine:componentParamEngine',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down Expand Up @@ -429,6 +446,8 @@ if (!EMBER_ROUTING_MODEL_ARG) {
'engine:paramEngine',
Engine.extend({
router: null,
Resolver: ModuleBasedTestResolver,

init() {
this._super(...arguments);
this.register(
Expand Down
14 changes: 13 additions & 1 deletion packages/@ember/application/globals-resolver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import { dictionary } from '@ember/-internals/utils';
import { get, findNamespace } from '@ember/-internals/metal';
import { assert, info } from '@ember/debug';
import { assert, info, deprecate } from '@ember/debug';
import { capitalize, classify, dasherize, decamelize } from '@ember/string';
import { Object as EmberObject } from '@ember/-internals/runtime';
import { getTemplate } from '@ember/-internals/glimmer';
Expand Down Expand Up @@ -77,6 +77,7 @@ import { DEBUG } from '@glimmer/env';
@class GlobalsResolver
@extends EmberObject
@public
@deprecated
*/

class DefaultResolver extends EmberObject {
Expand All @@ -93,9 +94,20 @@ class DefaultResolver extends EmberObject {

@property namespace
@public
@deprecated
*/

init() {
deprecate(
'Using the globals resolver is deprecated. Use the ember-resolver package instead. See https://deprecations.emberjs.com/v3.x#toc_ember-deprecate-globals-resolver',
false,
{
until: '4.0.0',
id: 'globals-resolver',
url: 'https://deprecations.emberjs.com/v3.x#toc_ember-deprecate-globals-resolver',
}
);

this._parseNameCache = dictionary(null);
}

Expand Down
18 changes: 15 additions & 3 deletions packages/@ember/application/tests/application_instance_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import { run } from '@ember/runloop';
import { privatize as P } from '@ember/-internals/container';
import { factory } from 'internal-test-helpers';
import { Object as EmberObject } from '@ember/-internals/runtime';
import { moduleFor, AbstractTestCase as TestCase } from 'internal-test-helpers';
import {
moduleFor,
ModuleBasedTestResolver,
AbstractTestCase as TestCase,
} from 'internal-test-helpers';
import { getDebugFunction, setDebugFunction } from '@ember/debug';

const originalDebug = getDebugFunction('debug');
Expand All @@ -23,7 +27,13 @@ moduleFor(
document.getElementById('qunit-fixture').innerHTML = `
<div id='one'><div id='one-child'>HI</div></div><div id='two'>HI</div>
`;
application = run(() => Application.create({ rootElement: '#one', router: null }));
application = run(() =>
Application.create({
rootElement: '#one',
router: null,
Resolver: ModuleBasedTestResolver,
})
);
}

teardown() {
Expand Down Expand Up @@ -167,7 +177,9 @@ moduleFor(
['@test can build and boot a registered engine'](assert) {
assert.expect(11);

let ChatEngine = Engine.extend();
let ChatEngine = Engine.extend({
Resolver: ModuleBasedTestResolver,
});
let chatEngineInstance;

application.register('engine:chat', ChatEngine);
Expand Down
4 changes: 4 additions & 0 deletions packages/@ember/application/tests/dependency_injection/default_resolver_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ moduleFor(
}

[`@test no assertion for routes that extend from Route`](assert) {
assert.test.assertions = []; // clear assertions that occurred in beforeEach
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, why do we need to clear the assertions?

Copy link
Contributor Author

@Gaurav0 Gaurav0 Nov 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

In beforeEach we are creaing an application using the default resolver. This throws off the following assert.expect statements.

assert.expect(0);
this.application.FooRoute = Route.extend();
this.privateRegistry.resolve(`route:foo`);
Expand All @@ -211,6 +212,7 @@ moduleFor(
}

[`@test no assertion for service factories that extend from Service`](assert) {
assert.test.assertions = []; // clear assertions that occurred in beforeEach
assert.expect(0);
this.application.FooService = Service.extend();
this.privateRegistry.resolve('service:foo');
Expand Down Expand Up @@ -309,6 +311,7 @@ moduleFor(
return;
}

assert.test.assertions = []; // clear assertions that occurred in beforeEach
assert.expect(3);

this.application.LOG_RESOLVER = true;
Expand All @@ -330,6 +333,7 @@ moduleFor(
return;
}

assert.test.assertions = []; // clear assertions that occurred in beforeEach
assert.expect(3);

this.application.LOG_RESOLVER = true;
Expand Down
7 changes: 6 additions & 1 deletion packages/@ember/application/tests/dependency_injection/normalization_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ moduleFor(
constructor() {
super();

application = run(Application, 'create');
// Must use default resolver because test resolver does not normalize
Gaurav0 marked this conversation as resolved.
Show resolved Hide resolved
run(() => {
expectDeprecation(() => {
application = Application.create();
});
});
registry = application.__registry__;
}

Expand Down
7 changes: 6 additions & 1 deletion packages/@ember/application/tests/dependency_injection_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ moduleFor(
constructor() {
super();

application = run(EmberApplication, 'create');
// Must use default resolver because test resolver does not normalize
Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, so lets split the tests that rely on the globals resolver into their own moduleFor + class (e.g. moduleFor('Application Dependency Injection - Globals Resolver [DEPRECATED]', class extends TestCase {})).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's only one test that relies on it, so I'm just going to move it into default_resolver_test.js.

run(() => {
expectDeprecation(() => {
application = EmberApplication.create();
});
});

application.Person = EmberObject.extend({});
application.Orange = EmberObject.extend({});
Expand Down
3 changes: 2 additions & 1 deletion packages/@ember/application/tests/readiness_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { moduleFor, ApplicationTestCase } from 'internal-test-helpers';
import { moduleFor, ModuleBasedTestResolver, ApplicationTestCase } from 'internal-test-helpers';
import { run } from '@ember/runloop';
import EmberApplication from '..';

Expand Down Expand Up @@ -45,6 +45,7 @@ moduleFor(

Application = EmberApplication.extend({
$: jQuery,
Resolver: ModuleBasedTestResolver,

ready() {
readyWasCalled++;
Expand Down
17 changes: 15 additions & 2 deletions packages/@ember/application/tests/visit_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { moduleFor, ApplicationTestCase, runTask } from 'internal-test-helpers';
import {
moduleFor,
ModuleBasedTestResolver,
ApplicationTestCase,
runTask,
} from 'internal-test-helpers';
import { inject as injectService } from '@ember/service';
import { Object as EmberObject, RSVP, onerrorDefault } from '@ember/-internals/runtime';
import { later } from '@ember/runloop';
Expand Down Expand Up @@ -465,7 +470,9 @@ moduleFor(
this.addTemplate('application', '<h1>Hello world</h1>');

// Register engine
let BlogEngine = Engine.extend();
let BlogEngine = Engine.extend({
Resolver: ModuleBasedTestResolver,
});
this.add('engine:blog', BlogEngine);

// Register engine route map
Expand Down Expand Up @@ -502,6 +509,8 @@ moduleFor(

// Register engine
let BlogEngine = Engine.extend({
Resolver: ModuleBasedTestResolver,

init(...args) {
this._super.apply(this, args);
this.register('template:application', compile('{{cache-money}}'));
Expand Down Expand Up @@ -544,6 +553,8 @@ moduleFor(

// Register engine
let BlogEngine = Engine.extend({
Resolver: ModuleBasedTestResolver,

init(...args) {
this._super.apply(this, args);
this.register('template:application', compile('{{cache-money}}'));
Expand Down Expand Up @@ -582,6 +593,8 @@ moduleFor(

// Register engine
let BlogEngine = Engine.extend({
Resolver: ModuleBasedTestResolver,

init(...args) {
this._super.apply(this, args);
this.register('template:application', compile('{{swag}}'));
Expand Down
Loading