From bd0c89812d35984b59238b838aaaae0668348610 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Wed, 17 Mar 2021 17:01:02 -0700 Subject: [PATCH] [FEATURE] Remove globals namespace based toString Removes the legacy `toString` behavior and replaces it with using the factory for/container system for classes when possible, and just `unknown` when not. --- .../-internals/container/lib/container.ts | 10 +- .../-internals/container/lib/registry.ts | 2 +- packages/@ember/-internals/metal/index.ts | 1 - packages/@ember/-internals/metal/lib/mixin.ts | 4 +- .../-internals/metal/lib/namespace_search.ts | 43 +-------- .../metal/tests/namespace_search_test.js | 2 +- packages/@ember/-internals/owner/index.ts | 1 + .../routing/tests/system/route_test.js | 2 +- .../runtime/lib/system/core_object.js | 28 +++--- .../-internals/runtime/lib/system/object.js | 4 +- .../tests/system/namespace/base_test.js | 28 +++--- .../tests/system/object/toString_test.js | 95 +------------------ .../@ember/application/globals-resolver.js | 9 +- .../application/tests/application_test.js | 6 +- .../dependency_injection/to_string_test.js | 72 -------------- packages/@ember/engine/tests/engine_test.js | 9 +- 16 files changed, 62 insertions(+), 254 deletions(-) delete mode 100644 packages/@ember/application/tests/dependency_injection/to_string_test.js diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 5d10fb8f106..e404f6b6357 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/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, enumerableSymbol, 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'; @@ -515,13 +515,13 @@ declare interface DebugFactory extends Factory { _lazyInjections(): { [key: string]: LazyInjection }; } -export const INIT_FACTORY = symbol('INIT_FACTORY'); +export const INIT_FACTORY = enumerableSymbol('INIT_FACTORY'); export function getFactoryFor(obj: any): FactoryManager { return obj[INIT_FACTORY]; } -export function setFactoryFor(obj: any, factory: FactoryManager) { +export function setFactoryFor(obj: any, factory: FactoryManager): void { obj[INIT_FACTORY] = factory; } @@ -548,6 +548,10 @@ class FactoryManager { this.madeToString = undefined; this.injections = undefined; setFactoryFor(this, this); + + if (factory && (HAS_NATIVE_SYMBOL || INIT_FACTORY in factory)) { + setFactoryFor(factory, this); + } } toString(): string { diff --git a/packages/@ember/-internals/container/lib/registry.ts b/packages/@ember/-internals/container/lib/registry.ts index 4e657e5544e..8659ac9e2f1 100644 --- a/packages/@ember/-internals/container/lib/registry.ts +++ b/packages/@ember/-internals/container/lib/registry.ts @@ -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)'; } } diff --git a/packages/@ember/-internals/metal/index.ts b/packages/@ember/-internals/metal/index.ts index ab62c3cae84..37fba150cb2 100644 --- a/packages/@ember/-internals/metal/index.ts +++ b/packages/@ember/-internals/metal/index.ts @@ -60,7 +60,6 @@ export { NAMESPACES, NAMESPACES_BY_ID, addNamespace, - classToString, findNamespace, findNamespaces, processNamespace, diff --git a/packages/@ember/-internals/metal/lib/mixin.ts b/packages/@ember/-internals/metal/lib/mixin.ts index c52c31e6640..e2f54685001 100644 --- a/packages/@ember/-internals/metal/lib/mixin.ts +++ b/packages/@ember/-internals/metal/lib/mixin.ts @@ -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'; @@ -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); } diff --git a/packages/@ember/-internals/metal/lib/namespace_search.ts b/packages/@ember/-internals/metal/lib/namespace_search.ts index 2d076c10c0f..356cb85dc77 100644 --- a/packages/@ember/-internals/metal/lib/namespace_search.ts +++ b/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; @@ -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; } @@ -139,7 +124,7 @@ function _processNamespace(paths: string[], root: Namespace, seen: Set { class?: C; + name?: string; fullName?: string; normalizedName?: string; create(props?: { [prop: string]: any }): T; diff --git a/packages/@ember/-internals/routing/tests/system/route_test.js b/packages/@ember/-internals/routing/tests/system/route_test.js index 537a57e0454..bcf4bfa32a5 100644 --- a/packages/@ember/-internals/routing/tests/system/route_test.js +++ b/packages/@ember/-internals/routing/tests/system/route_test.js @@ -124,7 +124,7 @@ moduleFor( expectAssertion(function () { route.model({ post_id: 1 }); - }, /You used the dynamic segment post_id in your route undefined, but .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); } diff --git a/packages/@ember/-internals/runtime/lib/system/core_object.js b/packages/@ember/-internals/runtime/lib/system/core_object.js index bc24e5dfbfa..c2262dbb450 100644 --- a/packages/@ember/-internals/runtime/lib/system/core_object.js +++ b/packages/@ember/-internals/runtime/lib/system/core_object.js @@ -7,8 +7,6 @@ import { getOwner, LEGACY_OWNER } from '@ember/-internals/owner'; import { assign } from '@ember/polyfills'; import { guidFor, - getName, - setName, lookupDescriptor, inspect, makeArray, @@ -25,7 +23,6 @@ import { applyMixin, defineProperty, descriptorForProperty, - classToString, isClassicDecorator, DEBUG_INJECTION_FUNCTIONS, TrackedDescriptor, @@ -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}>`; } /** @@ -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; @@ -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) { diff --git a/packages/@ember/-internals/runtime/lib/system/object.js b/packages/@ember/-internals/runtime/lib/system/object.js index e789731c3fc..5f29bc2bf06 100644 --- a/packages/@ember/-internals/runtime/lib/system/object.js +++ b/packages/@ember/-internals/runtime/lib/system/object.js @@ -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'; @@ -27,8 +27,6 @@ export default class EmberObject extends CoreObject { } } -setName(EmberObject, 'Ember.Object'); - Observable.apply(EmberObject.prototype); export let FrameworkObject; diff --git a/packages/@ember/-internals/runtime/tests/system/namespace/base_test.js b/packages/@ember/-internals/runtime/tests/system/namespace/base_test.js index c791f2bd173..5c721d6b3a5 100644 --- a/packages/@ember/-internals/runtime/tests/system/namespace/base_test.js +++ b/packages/@ember/-internals/runtime/tests/system/namespace/base_test.js @@ -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'; @@ -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' ); @@ -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) { @@ -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" ); @@ -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) { diff --git a/packages/@ember/-internals/runtime/tests/system/object/toString_test.js b/packages/@ember/-internals/runtime/tests/system/object/toString_test.js index 1e612de956e..65d6ac5deba 100644 --- a/packages/@ember/-internals/runtime/tests/system/object/toString_test.js +++ b/packages/@ember/-internals/runtime/tests/system/object/toString_test.js @@ -1,101 +1,10 @@ -import { run } from '@ember/runloop'; import { guidFor, setName } from '@ember/-internals/utils'; -import { context } from '@ember/-internals/environment'; import EmberObject from '../../../lib/system/object'; -import Namespace from '../../../lib/system/namespace'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; -let originalLookup = context.lookup; -let lookup; - moduleFor( 'system/object/toString', class extends AbstractTestCase { - beforeEach() { - context.lookup = lookup = {}; - } - - afterEach() { - context.lookup = originalLookup; - } - - ['@test toString() returns the same value if called twice'](assert) { - let Foo = Namespace.create(); - Foo.toString = function () { - return 'Foo'; - }; - - Foo.Bar = EmberObject.extend(); - - assert.equal(Foo.Bar.toString(), 'Foo.Bar'); - assert.equal(Foo.Bar.toString(), 'Foo.Bar'); - - let obj = Foo.Bar.create(); - - assert.equal(obj.toString(), ''); - assert.equal(obj.toString(), ''); - - assert.equal(Foo.Bar.toString(), 'Foo.Bar'); - - run(Foo, 'destroy'); - } - - ['@test toString on a class returns a useful value when nested in a namespace'](assert) { - let obj; - - let Foo = Namespace.create(); - Foo.toString = function () { - return 'Foo'; - }; - - Foo.Bar = EmberObject.extend(); - assert.equal(Foo.Bar.toString(), 'Foo.Bar'); - - obj = Foo.Bar.create(); - assert.equal(obj.toString(), ''); - - Foo.Baz = Foo.Bar.extend(); - assert.equal(Foo.Baz.toString(), 'Foo.Baz'); - - obj = Foo.Baz.create(); - assert.equal(obj.toString(), ''); - - obj = Foo.Bar.create(); - assert.equal(obj.toString(), ''); - - run(Foo, 'destroy'); - } - - ['@test toString on a namespace finds the namespace in lookup'](assert) { - let Foo = (lookup.Foo = Namespace.create()); - - assert.equal(Foo.toString(), 'Foo'); - - run(Foo, 'destroy'); - } - - ['@test toString on a nested namespace finds the namespace in lookup'](assert) { - let Foo = (lookup.Foo = Namespace.create()); - let obj; - - Foo.Bar = EmberObject.extend(); - - assert.equal(Foo.Bar.toString(), 'Foo.Bar'); - - obj = Foo.Bar.create(); - assert.equal(obj.toString(), ''); - - run(Foo, 'destroy'); - } - - ['@test toString on a namespace falls back to modulePrefix, if defined'](assert) { - let Foo = Namespace.create({ modulePrefix: 'foo' }); - - assert.equal(Foo.toString(), 'foo'); - - run(Foo, 'destroy'); - } - ['@test toString includes toStringExtension if defined'](assert) { let Foo = EmberObject.extend({ toStringExtension() { @@ -112,12 +21,12 @@ moduleFor( assert.equal( bar.toString(), - '', + '<(unknown):' + guidFor(bar) + '>', 'does not include toStringExtension part' ); assert.equal( foo.toString(), - '', + '<(unknown):' + guidFor(foo) + ':fooey>', 'Includes toStringExtension result' ); } diff --git a/packages/@ember/application/globals-resolver.js b/packages/@ember/application/globals-resolver.js index 6cb9987131b..67cad755c4d 100644 --- a/packages/@ember/application/globals-resolver.js +++ b/packages/@ember/application/globals-resolver.js @@ -2,7 +2,7 @@ @module @ember/application */ -import { dictionary } from '@ember/-internals/utils'; +import { dictionary, getName } from '@ember/-internals/utils'; import { get, findNamespace } from '@ember/-internals/metal'; import { assert, info, deprecate } from '@ember/debug'; import { capitalize, classify, dasherize, decamelize } from '@ember/string'; @@ -10,6 +10,7 @@ import { Object as EmberObject } from '@ember/-internals/runtime'; import { getTemplate } from '@ember/-internals/glimmer'; import { DEBUG } from '@glimmer/env'; import { GLOBALS_RESOLVER } from '@ember/deprecated-features'; +import { processAllNamespaces } from '@ember/-internals/metal'; /** The DefaultResolver defines the default lookup rules to resolve @@ -177,9 +178,11 @@ if (GLOBALS_RESOLVER) { if (validationAttributes) { let [factoryFlag, expectedType] = validationAttributes; + processAllNamespaces(); + assert( `Expected ${parsedName.fullName} to resolve to an ${expectedType} but ` + - `instead it was ${resolved}.`, + `instead it was ${getName(resolved)}.`, Boolean(resolved[factoryFlag]) ); } @@ -274,7 +277,7 @@ if (GLOBALS_RESOLVER) { } makeToString(factory) { - return factory.toString(); + return typeof factory === 'string' ? factory : factory.name ?? '(unknown class)'; } /** diff --git a/packages/@ember/application/tests/application_test.js b/packages/@ember/application/tests/application_test.js index 713d8a78bae..ee39c6eb58a 100644 --- a/packages/@ember/application/tests/application_test.js +++ b/packages/@ember/application/tests/application_test.js @@ -1,7 +1,8 @@ import { DEBUG } from '@glimmer/env'; import VERSION from 'ember/version'; import { ENV, context } from '@ember/-internals/environment'; -import { libraries } from '@ember/-internals/metal'; +import { libraries, processAllNamespaces } from '@ember/-internals/metal'; +import { getName } from '@ember/-internals/utils'; import { getDebugFunction, setDebugFunction } from '@ember/debug'; import { Router, NoneLocation, Route as EmberRoute } from '@ember/-internals/routing'; import { jQueryDisabled, jQuery } from '@ember/-internals/views'; @@ -194,7 +195,8 @@ moduleFor( [`@test acts like a namespace`](assert) { this.application = runTask(() => this.createApplication()); let Foo = (this.application.Foo = EmberObject.extend()); - assert.equal(Foo.toString(), 'TestApp.Foo', 'Classes pick up their parent namespace'); + processAllNamespaces(); + assert.equal(getName(Foo), 'TestApp.Foo', 'Classes pick up their parent namespace'); } [`@test can specify custom router`](assert) { diff --git a/packages/@ember/application/tests/dependency_injection/to_string_test.js b/packages/@ember/application/tests/dependency_injection/to_string_test.js deleted file mode 100644 index b1e27d3a6bb..00000000000 --- a/packages/@ember/application/tests/dependency_injection/to_string_test.js +++ /dev/null @@ -1,72 +0,0 @@ -import { assign } from '@ember/polyfills'; -import { guidFor } from '@ember/-internals/utils'; -import { Object as EmberObject } from '@ember/-internals/runtime'; -import { - moduleFor, - ApplicationTestCase, - ModuleBasedTestResolver, - DefaultResolverApplicationTestCase, - runTask, -} from 'internal-test-helpers'; - -moduleFor( - 'Application Dependency Injection - DefaultResolver#toString', - class extends DefaultResolverApplicationTestCase { - constructor() { - super(); - runTask(() => this.createApplication()); - this.application.Post = EmberObject.extend(); - } - - beforeEach() { - return this.visit('/'); - } - - ['@test factories'](assert) { - let PostFactory = this.applicationInstance.factoryFor('model:post').class; - assert.equal(PostFactory.toString(), 'TestApp.Post', 'expecting the model to be post'); - } - - ['@test instances'](assert) { - let post = this.applicationInstance.lookup('model:post'); - let guid = guidFor(post); - - assert.equal( - post.toString(), - '', - 'expecting the model to be post' - ); - } - } -); - -moduleFor( - 'Application Dependency Injection - Resolver#toString', - class extends ApplicationTestCase { - beforeEach() { - return this.visit('/'); - } - - get applicationOptions() { - return assign(super.applicationOptions, { - Resolver: class extends ModuleBasedTestResolver { - makeToString(_, fullName) { - return fullName; - } - }, - }); - } - - ['@test toString called on a resolver'](assert) { - this.add('model:peter', EmberObject.extend()); - - let peter = this.applicationInstance.lookup('model:peter'); - let guid = guidFor(peter); - assert.equal( - peter.toString(), - ``, - 'expecting the supermodel to be peter' - ); - } - } -); diff --git a/packages/@ember/engine/tests/engine_test.js b/packages/@ember/engine/tests/engine_test.js index d5c5d1c0dfa..8507fcc4d0d 100644 --- a/packages/@ember/engine/tests/engine_test.js +++ b/packages/@ember/engine/tests/engine_test.js @@ -2,6 +2,8 @@ import { context } from '@ember/-internals/environment'; import { run } from '@ember/runloop'; import Engine from '@ember/engine'; import { Object as EmberObject } from '@ember/-internals/runtime'; +import { processAllNamespaces } from '@ember/-internals/metal'; +import { getName } from '@ember/-internals/utils'; import { moduleFor, AbstractTestCase as TestCase, @@ -37,11 +39,8 @@ moduleFor( ['@test acts like a namespace'](assert) { engine.Foo = EmberObject.extend(); - assert.equal( - engine.Foo.toString(), - 'TestEngine.Foo', - 'Classes pick up their parent namespace' - ); + processAllNamespaces(); + assert.equal(getName(engine.Foo), 'TestEngine.Foo', 'Classes pick up their parent namespace'); } ['@test builds a registry'](assert) {