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

[FEATURE] Remove globals namespace based toString #19467

Merged
merged 1 commit into from Mar 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/@ember/-internals/container/lib/container.ts
@@ -1,5 +1,5 @@
import { Factory, LookupOptions, Owner, setOwner } from '@ember/-internals/owner';
import { dictionary, HAS_NATIVE_PROXY, symbol } from '@ember/-internals/utils';
import { dictionary, HAS_NATIVE_PROXY, HAS_NATIVE_SYMBOL, symbol } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import { assign } from '@ember/polyfills';
import { DEBUG } from '@glimmer/env';
Expand Down Expand Up @@ -521,7 +521,7 @@ export function getFactoryFor(obj: any): FactoryManager<any, any> {
return obj[INIT_FACTORY];
}

export function setFactoryFor(obj: any, factory: FactoryManager<any, any>) {
export function setFactoryFor(obj: any, factory: FactoryManager<any, any>): void {
obj[INIT_FACTORY] = factory;
}

Expand All @@ -548,6 +548,10 @@ class FactoryManager<T, C> {
this.madeToString = undefined;
this.injections = undefined;
setFactoryFor(this, this);

if (factory && (HAS_NATIVE_SYMBOL || INIT_FACTORY in factory)) {
setFactoryFor(factory, this);
}
}

toString(): string {
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/-internals/container/lib/registry.ts
Expand Up @@ -362,7 +362,7 @@ export default class Registry implements IRegistry {
} else if (this.fallback !== null) {
return this.fallback.makeToString(factory, fullName);
} else {
return factory.toString();
return typeof factory === 'string' ? factory : factory.name ?? '(unknown class)';
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/@ember/-internals/metal/index.ts
Expand Up @@ -60,7 +60,6 @@ export {
NAMESPACES,
NAMESPACES_BY_ID,
addNamespace,
classToString,
findNamespace,
findNamespaces,
processNamespace,
Expand Down
4 changes: 1 addition & 3 deletions packages/@ember/-internals/metal/lib/mixin.ts
Expand Up @@ -32,7 +32,7 @@ import {
} from './decorator';
import { addListener, removeListener } from './events';
import expandProperties from './expand_properties';
import { classToString, setUnprocessedMixins } from './namespace_search';
import { setUnprocessedMixins } from './namespace_search';
import { addObserver, removeObserver, revalidateObservers } from './observer';
import { defineDecorator, defineValue } from './properties';

Expand Down Expand Up @@ -736,8 +736,6 @@ function buildMixinsArray(mixins: MixinLike[] | undefined): Mixin[] | undefined

type MixinLike = Mixin | { [key: string]: any };

Mixin.prototype.toString = classToString;

if (DEBUG) {
Object.seal(Mixin.prototype);
}
Expand Down
43 changes: 1 addition & 42 deletions packages/@ember/-internals/metal/lib/namespace_search.ts
@@ -1,11 +1,6 @@
import { context } from '@ember/-internals/environment';
import { getName, setName } from '@ember/-internals/utils';

// TODO, this only depends on context, otherwise it could be in utils
// move into its own package
// it is needed by Mixin for classToString
// maybe move it into environment

const hasOwnProperty = Object.prototype.hasOwnProperty;

let searchDisabled = false;
Expand Down Expand Up @@ -94,16 +89,6 @@ export function processAllNamespaces(): void {
}
}

export function classToString(this: object): string {
let name = getName(this);
if (name !== void 0) {
return name;
}
name = calculateToString(this);
setName(this, name);
return name;
}

export function isSearchDisabled(): boolean {
return searchDisabled;
}
Expand Down Expand Up @@ -139,7 +124,7 @@ function _processNamespace(paths: string[], root: Namespace, seen: Set<Namespace
paths[idx] = key;

// If we have found an unprocessed class
if (obj && obj.toString === classToString && getName(obj) === void 0) {
if (obj && getName(obj) === void 0) {
// Replace the class' `toString` with the dot-separated path
setName(obj, paths.join('.'));
// Support nested namespaces
Expand Down Expand Up @@ -175,29 +160,3 @@ function tryIsNamespace(lookup: { [k: string]: any }, prop: string): Namespace |
// continue
}
}

function calculateToString(target: object): string {
let str;

if (!searchDisabled) {
processAllNamespaces();

str = getName(target);
if (str !== void 0) {
return str;
}
let superclass = target;
do {
superclass = Object.getPrototypeOf(superclass);
if (superclass === Function.prototype || superclass === Object.prototype) {
break;
}
str = getName(target);
if (str !== void 0) {
str = `(subclass of ${str})`;
break;
}
} while (str === void 0);
}
return str || '(unknown)';
}
Expand Up @@ -8,7 +8,7 @@ moduleFor(
let mixin = Mixin.create();
assert.equal(
mixin.toString(),
'(unknown)',
'(unknown mixin)',
'this = null should be handled on Mixin.toString() call'
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/@ember/-internals/owner/index.ts
Expand Up @@ -15,6 +15,7 @@ export interface FactoryClass {

export interface Factory<T, C extends FactoryClass | object = FactoryClass> {
class?: C;
name?: string;
fullName?: string;
normalizedName?: string;
create(props?: { [prop: string]: any }): T;
Expand Down
Expand Up @@ -124,7 +124,7 @@ moduleFor(

expectAssertion(function () {
route.model({ post_id: 1 });
}, /You used the dynamic segment post_id in your route undefined, but <Ember.Object:ember\d+>.Post did not exist and you did not override your route's `model` hook./);
}, /You used the dynamic segment post_id in your route undefined, but <.*:ember\d+>.Post did not exist and you did not override your route's `model` hook./);

runDestroy(owner);
}
Expand Down
28 changes: 17 additions & 11 deletions packages/@ember/-internals/runtime/lib/system/core_object.js
Expand Up @@ -7,8 +7,6 @@ import { getOwner, LEGACY_OWNER } from '@ember/-internals/owner';
import { assign } from '@ember/polyfills';
import {
guidFor,
getName,
setName,
lookupDescriptor,
inspect,
makeArray,
Expand All @@ -25,7 +23,6 @@ import {
applyMixin,
defineProperty,
descriptorForProperty,
classToString,
isClassicDecorator,
DEBUG_INJECTION_FUNCTIONS,
TrackedDescriptor,
Expand Down Expand Up @@ -691,11 +688,7 @@ class CoreObject {
let hasToStringExtension = typeof this.toStringExtension === 'function';
let extension = hasToStringExtension ? `:${this.toStringExtension()}` : '';

let ret = `<${getName(this) || getFactoryFor(this) || this.constructor.toString()}:${guidFor(
this
)}${extension}>`;

return ret;
return `<${getFactoryFor(this) || '(unknown)'}:${guidFor(this)}${extension}>`;
}

/**
Expand Down Expand Up @@ -1092,10 +1085,11 @@ class CoreObject {
}
return p;
}
}

CoreObject.toString = classToString;
setName(CoreObject, 'Ember.CoreObject');
static toString() {
return `<${getFactoryFor(this) || '(unknown)'}:constructor>`;
}
}

CoreObject.isClass = true;
CoreObject.isMethod = false;
Expand Down Expand Up @@ -1223,6 +1217,18 @@ if (!HAS_NATIVE_SYMBOL) {
instanceFactory.set(this, value);
},
});

Object.defineProperty(CoreObject, INIT_FACTORY, {
get() {
return instanceFactory.get(this);
},

set(value) {
instanceFactory.set(this, value);
},

enumerable: false,
});
}

function implicitInjectionDeprecation(keyName, msg = null) {
Expand Down
4 changes: 1 addition & 3 deletions packages/@ember/-internals/runtime/lib/system/object.js
Expand Up @@ -3,7 +3,7 @@
*/

import { getFactoryFor } from '@ember/-internals/container';
import { symbol, setName } from '@ember/-internals/utils';
import { symbol } from '@ember/-internals/utils';
import { addListener } from '@ember/-internals/metal';
import CoreObject from './core_object';
import Observable from '../mixins/observable';
Expand All @@ -27,8 +27,6 @@ export default class EmberObject extends CoreObject {
}
}

setName(EmberObject, 'Ember.Object');

Observable.apply(EmberObject.prototype);

export let FrameworkObject;
Expand Down
@@ -1,7 +1,7 @@
import { context } from '@ember/-internals/environment';
import { run } from '@ember/runloop';
import { get, setNamespaceSearchDisabled } from '@ember/-internals/metal';
import { guidFor } from '@ember/-internals/utils';
import { guidFor, getName } from '@ember/-internals/utils';
import EmberObject from '../../../lib/system/object';
import Namespace from '../../../lib/system/namespace';
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
Expand Down Expand Up @@ -62,19 +62,18 @@ moduleFor(
['@test Classes under an Namespace are properly named'](assert) {
let nsA = (lookup.NamespaceA = Namespace.create());
nsA.Foo = EmberObject.extend();
assert.equal(nsA.Foo.toString(), 'NamespaceA.Foo', 'Classes pick up their parent namespace');
Namespace.processAll();
assert.equal(getName(nsA.Foo), 'NamespaceA.Foo', 'Classes pick up their parent namespace');

nsA.Bar = EmberObject.extend();
assert.equal(
nsA.Bar.toString(),
'NamespaceA.Bar',
'New Classes get the naming treatment too'
);
Namespace.processAll();
assert.equal(getName(nsA.Bar), 'NamespaceA.Bar', 'New Classes get the naming treatment too');

let nsB = (lookup.NamespaceB = Namespace.create());
nsB.Foo = EmberObject.extend();
Namespace.processAll();
assert.equal(
nsB.Foo.toString(),
getName(nsB.Foo),
'NamespaceB.Foo',
'Classes in new namespaces get the naming treatment'
);
Expand All @@ -88,7 +87,8 @@ moduleFor(

['@test Lowercase namespaces are no longer supported'](assert) {
let nsC = (lookup.namespaceC = Namespace.create());
assert.equal(nsC.toString(), guidFor(nsC));
Namespace.processAll();
assert.equal(getName(nsC), guidFor(nsC));
}

['@test A namespace can be assigned a custom name'](assert) {
Expand All @@ -104,13 +104,15 @@ moduleFor(
nsA.Foo = EmberObject.extend();
nsB.Foo = EmberObject.extend();

Namespace.processAll();

assert.equal(
nsA.Foo.toString(),
getName(nsA.Foo),
'NamespaceA.Foo',
"The namespace's name is used when the namespace is not in the lookup object"
);
assert.equal(
nsB.Foo.toString(),
getName(nsB.Foo),
'CustomNamespaceB.Foo',
"The namespace's name is used when the namespace is in the lookup object"
);
Expand All @@ -129,8 +131,8 @@ moduleFor(

Namespace.processAll();

assert.equal(namespace.ClassA.toString(), 'NS.ClassA');
assert.equal(namespace.ClassB.toString(), 'NS.ClassB');
assert.equal(getName(namespace.ClassA), 'NS.ClassA');
assert.equal(getName(namespace.ClassB), 'NS.ClassB');
}

['@test A namespace can be looked up by its name'](assert) {
Expand Down