Merge pull request #16615 from emberjs/fix-namespace-destroy

[BUGFIX release] Fix NAMESPACES_BY_ID leaks

packages/@ember/application/tests/application_test.js

@@ -1,7 +1,7 @@
 /*globals EmberDev */
 import VERSION from 'ember/version';
 import { ENV, context } from 'ember-environment';
-import { libraries, setNamespaceSearchDisabled } from 'ember-metal';
+import { libraries } from 'ember-metal';
 import { getDebugFunction, setDebugFunction } from '@ember/debug';
 import Application from '..';
 import { Router, NoneLocation, Route as EmberRoute } from 'ember-routing';
@@ -232,11 +232,7 @@ moduleFor(
     }
 
     [`@test acts like a namespace`](assert) {
-      let lookup = (context.lookup = {});
-
-      lookup.TestApp = this.application = this.runTask(() => this.createApplication());
-
-      setNamespaceSearchDisabled(false);
+      this.application = this.runTask(() => this.createApplication());
       let Foo = (this.application.Foo = EmberObject.extend());
       assert.equal(Foo.toString(), 'TestApp.Foo', 'Classes pick up their parent namespace');
     }

packages/@ember/application/tests/dependency_injection/default_resolver_test.js

@@ -206,7 +206,7 @@ moduleFor(
       expectAssertion(() => {
         this.application.FooService = EmberObject.extend();
         this.privateRegistry.resolve('service:foo');
-      }, /Expected service:foo to resolve to an Ember.Service but instead it was \.FooService\./);
+      }, /Expected service:foo to resolve to an Ember.Service but instead it was TestApp\.FooService\./);
     }
 
     [`@test no deprecation warning for service factories that extend from Service`](assert) {
@@ -219,7 +219,7 @@ moduleFor(
       expectAssertion(() => {
         this.application.FooComponent = EmberObject.extend();
         this.privateRegistry.resolve('component:foo');
-      }, /Expected component:foo to resolve to an Ember\.Component but instead it was \.FooComponent\./);
+      }, /Expected component:foo to resolve to an Ember\.Component but instead it was TestApp\.FooComponent\./);
     }
 
     [`@test no deprecation warning for component factories that extend from Component`]() {

packages/@ember/application/tests/dependency_injection/to_string_test.js

@@ -23,14 +23,18 @@ moduleFor(
 
     ['@test factories'](assert) {
       let PostFactory = this.applicationInstance.factoryFor('model:post').class;
-      assert.equal(PostFactory.toString(), '.Post', 'expecting the model to be post');
+      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(), '<.Post:' + guid + '>', 'expecting the model to be post');
+      assert.equal(
+        post.toString(),
+        '<TestApp.Post:' + guid + '>',
+        'expecting the model to be post'
+      );
     }
   }
 );

packages/ember-metal/lib/namespace_search.js

@@ -33,15 +33,12 @@ export function addNamespace(namespace) {
 }
 
 export function removeNamespace(namespace) {
-  let id = namespace.toString();
-  if (id) {
-    delete NAMESPACES_BY_ID[id];
-    if (namespace === context.lookup[id]) {
-      context.lookup[id] = undefined;
-    }
-  }
-
+  let name = getName(namespace);
+  delete NAMESPACES_BY_ID[name];
   NAMESPACES.splice(NAMESPACES.indexOf(namespace), 1);
+  if (name in context.lookup && namespace === context.lookup[name]) {
+    context.lookup[name] = undefined;
+  }
 }
 
 export function findNamespaces() {
@@ -117,7 +114,10 @@ export function setUnprocessedMixins() {
 function _processNamespace(paths, root, seen) {
   let idx = paths.length;
 
-  NAMESPACES_BY_ID[paths.join('.')] = root;
+  let id = paths.join('.');
+
+  NAMESPACES_BY_ID[id] = root;
+  setName(root, id);
 
   // Loop over all of the keys in the namespace, looking for classes
   for (let key in root) {
@@ -144,7 +144,6 @@ function _processNamespace(paths, root, seen) {
         continue;
       }
       seen.add(obj);
-
       // Process the child namespace
       _processNamespace(paths, obj, seen);
     }

packages/ember-runtime/lib/system/namespace.js

@@ -12,7 +12,7 @@ import {
   processAllNamespaces,
   removeNamespace,
 } from 'ember-metal'; // Preloaded into namespaces
-import { getName } from 'ember-utils';
+import { getName, guidFor, setName } from 'ember-utils';
 import EmberObject from './object';
 
 /**
@@ -45,9 +45,13 @@ const Namespace = EmberObject.extend({
     if (name) {
       return name;
     }
-
     findNamespaces();
-    return getName(this);
+    name = getName(this);
+    if (name === undefined) {
+      name = guidFor(this);
+      setName(this, name);
+    }
+    return name;
   },
 
   nameClasses() {

packages/ember-runtime/tests/system/namespace/base_test.js

@@ -1,6 +1,7 @@
 import { context } from 'ember-environment';
 import { run } from '@ember/runloop';
 import { get, setNamespaceSearchDisabled } from 'ember-metal';
+import { guidFor } from 'ember-utils';
 import EmberObject from '../../../lib/system/object';
 import Namespace from '../../../lib/system/namespace';
 import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
@@ -87,7 +88,7 @@ moduleFor(
 
     ['@test Lowercase namespaces are no longer supported'](assert) {
       let nsC = (lookup.namespaceC = Namespace.create());
-      assert.equal(nsC.toString(), undefined);
+      assert.equal(nsC.toString(), guidFor(nsC));
     }
 
     ['@test A namespace can be assigned a custom name'](assert) {

packages/internal-test-helpers/lib/ember-dev/namespaces.js

@@ -1,5 +1,5 @@
 import { run } from '@ember/runloop';
-import { NAMESPACES } from 'ember-metal';
+import { NAMESPACES, NAMESPACES_BY_ID } from 'ember-metal';
 
 function NamespacesAssert(env) {
   this.env = env;
@@ -20,6 +20,13 @@ NamespacesAssert.prototype = {
         }
       });
     }
+    let keys = Object.keys(NAMESPACES_BY_ID);
+    if (keys.length > 0) {
+      assert.ok(false, 'Should not have any NAMESPACES_BY_ID after tests');
+      for (let i = 0; i < keys.length; i++) {
+        delete NAMESPACES_BY_ID[keys[i]];
+      }
+    }
   },
   restore: function() {},
 };

packages/internal-test-helpers/lib/test-cases/default-resolver-application.js

@@ -14,6 +14,7 @@ export default class ApplicationTestCase extends AbstractApplicationTestCase {
 
   get applicationOptions() {
     return assign(super.applicationOptions, {
+      name: 'TestApp',
       autoboot: false,
       Resolver: DefaultResolver,
     });