From 0b154e79b45a42375817bc825d96d0ba8d0b6509 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 16 Mar 2022 08:10:38 -0600 Subject: [PATCH] Correctly associate props with factory and owner in FactoryManager Previously, we stomped the `props` binding with an `Object.assign()`, which meant that the original empty props object would get GC'd after the end of the method and the item passed into the class created at the end of the `FactoryManager.create` call would be a *different* object, which does *not* have the factory or owner associations. Fixes #20023 --- packages/@ember/-internals/container/lib/container.ts | 6 +----- .../@ember/-internals/routing/lib/services/routing.ts | 2 +- .../@ember/-internals/routing/lib/system/router.ts | 10 ++++++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 9911051d29b..36e6ba1293f 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -504,14 +504,10 @@ export class FactoryManager { ); } - let props = {}; + let props = options ? { ...options } : {}; setOwner(props, container.owner!); setFactoryFor(props, this); - if (options !== undefined) { - props = Object.assign({}, props, options); - } - if (DEBUG) { let lazyInjections; let validationCache = this.container.validationCache; diff --git a/packages/@ember/-internals/routing/lib/services/routing.ts b/packages/@ember/-internals/routing/lib/services/routing.ts index 081ee17d9c9..514939114c7 100644 --- a/packages/@ember/-internals/routing/lib/services/routing.ts +++ b/packages/@ember/-internals/routing/lib/services/routing.ts @@ -12,7 +12,7 @@ import { Route } from '../..'; import EmberRouter from '../system/router'; import RouterState from '../system/router_state'; -const ROUTER = (symbol('ROUTER') as unknown) as string; +const ROUTER = symbol('ROUTER'); /** The Routing service is used by LinkTo, and provides facilities for diff --git a/packages/@ember/-internals/routing/lib/system/router.ts b/packages/@ember/-internals/routing/lib/system/router.ts index 7b0c6ae8d20..96a6f41f871 100644 --- a/packages/@ember/-internals/routing/lib/system/router.ts +++ b/packages/@ember/-internals/routing/lib/system/router.ts @@ -341,7 +341,7 @@ class EmberRouter extends EmberObject.extend(Evented) i assert('Route is unexpectedly missing an owner', routeOwner); - let route = routeOwner.lookup(fullRouteName) as R; + let route = routeOwner.lookup(fullRouteName) as R | undefined; if (seen[name]) { assert('seen routes should exist', route); @@ -351,6 +351,8 @@ class EmberRouter extends EmberObject.extend(Evented) i seen[name] = true; if (!route) { + // SAFETY: this is configured in `commonSetupRegistry` in the + // `@ember/application/lib` package. let DefaultRoute: any = routeOwner.factoryFor('route:basic')!.class; routeOwner.register(fullRouteName, DefaultRoute.extend()); route = routeOwner.lookup(fullRouteName) as R; @@ -362,15 +364,15 @@ class EmberRouter extends EmberObject.extend(Evented) i } } - route!._setRouteName(routeName); + route._setRouteName(routeName); - if (engineInfo && !hasDefaultSerialize(route!)) { + if (engineInfo && !hasDefaultSerialize(route)) { throw new Error( 'Defining a custom serialize method on an Engine route is not supported.' ); } - return route!; + return route; } getSerializer(name: string) {