[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.
emberjs · Mar 18, 2021 · a80b54e · a80b54e
1 parent 2ab7a67
commit a80b54e
Show file tree

Hide file tree

Showing 16 changed files with 61 additions and 253 deletions.
diff --git 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, 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';
@@ -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;
 }
 
@@ -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 {

diff --git 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
@@ -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
@@ -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
@@ -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<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
@@ -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)';
-}
diff --git a/packages/@ember/-internals/metal/tests/namespace_search_test.js b/packages/@ember/-internals/metal/tests/namespace_search_test.js
@@ -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'
       );
     }

diff --git a/packages/@ember/-internals/owner/index.ts b/packages/@ember/-internals/owner/index.ts
@@ -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;

diff --git 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 <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);
     }

diff --git 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
@@ -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
@@ -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) {