fix(ivy): align NgModule registration timing with ViewEngine (#30244)

Currently in Ivy `NgModule` registration happens when the class is declared, however this is inconsistent with ViewEngine and requires extra generated code. These changes remove the generated code for `registerModuleFactory`, pass the id through to the `ngModuleDef` and do the module registration inside `NgModuleFactory.create`.

This PR resolves FW-1285.

PR Close #30244
This commit is contained in:
Kristiyan Kostadinov 2019-05-07 22:57:55 -04:00 committed by Alex Rickabaugh
parent 2f35dbfd3b
commit f74373f2dd
18 changed files with 115 additions and 73 deletions

View File

@ -162,6 +162,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
exports,
imports,
containsForwardDecls,
id,
emitInline: false,
// TODO: to be implemented as a part of FW-1004.
schemas: [],
@ -257,11 +258,6 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
ngModuleStatements.push(callExpr.toStmt());
}
}
if (analysis.id !== null) {
const registerNgModuleType = new ExternalExpr(R3Identifiers.registerNgModuleType);
const callExpr = registerNgModuleType.callFn([analysis.id, new WrappedNodeExpr(node.name)]);
ngModuleStatements.push(callExpr.toStmt());
}
return [
{
name: 'ngModuleDef',

View File

@ -484,7 +484,7 @@ describe('ngtsc behavioral tests', () => {
expect(jsContents).not.toContain('\u0275\u0275setNgModuleScope(TestModule,');
});
it('should emit a \u0275registerNgModuleType call when the module has an id', () => {
it('should emit the id when the module\'s id is a string', () => {
env.tsconfig();
env.write('test.ts', `
import {NgModule} from '@angular/core';
@ -496,27 +496,27 @@ describe('ngtsc behavioral tests', () => {
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('i0.\u0275registerNgModuleType(\'test\', TestModule);');
expect(jsContents).toContain(`i0.\u0275\u0275defineNgModule({ type: TestModule, id: 'test' })`);
});
it('should emit a \u0275registerNgModuleType call when the module id is defined as `module.id`',
() => {
env.tsconfig();
env.write('index.d.ts', `
it('should emit the id when the module\'s id is defined as `module.id`', () => {
env.tsconfig();
env.write('index.d.ts', `
declare const module = {id: string};
`);
env.write('test.ts', `
env.write('test.ts', `
import {NgModule} from '@angular/core';
@NgModule({id: module.id})
export class TestModule {}
`);
env.driveMain();
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('i0.\u0275registerNgModuleType(module.id, TestModule);');
});
const jsContents = env.getContents('test.js');
expect(jsContents)
.toContain('i0.\u0275\u0275defineNgModule({ type: TestModule, id: module.id })');
});
it('should filter out directives and pipes from module exports in the injector def', () => {
env.tsconfig();

View File

@ -106,6 +106,7 @@ export interface R3NgModuleMetadataFacade {
exports: Function[];
emitInline: boolean;
schemas: {name: string}[]|null;
id: string|null;
}
export interface R3InjectorMetadataFacade {

View File

@ -91,6 +91,7 @@ export class CompilerFacadeImpl implements CompilerFacade {
emitInline: true,
containsForwardDecls: false,
schemas: facade.schemas ? facade.schemas.map(wrapReference) : null,
id: facade.id ? new WrappedNodeExpr(facade.id) : null,
};
const res = compileNgModule(meta);
return this.jitExpression(res.expression, angularCoreEnv, sourceMapUrl, []);

View File

@ -239,9 +239,6 @@ export class Identifiers {
moduleName: CORE,
};
static registerNgModuleType:
o.ExternalReference = {name: 'ɵregisterNgModuleType', moduleName: CORE};
// sanitization-related functions
static sanitizeHtml: o.ExternalReference = {name: 'ɵɵsanitizeHtml', moduleName: CORE};
static sanitizeStyle: o.ExternalReference = {name: 'ɵɵsanitizeStyle', moduleName: CORE};

View File

@ -67,6 +67,9 @@ export interface R3NgModuleMetadata {
* The set of schemas that declare elements to be allowed in the NgModule.
*/
schemas: R3Reference[]|null;
/** Unique ID or expression representing the unique ID of an NgModule. */
id: o.Expression|null;
}
/**
@ -81,7 +84,8 @@ export function compileNgModule(meta: R3NgModuleMetadata): R3NgModuleDef {
exports,
schemas,
containsForwardDecls,
emitInline
emitInline,
id
} = meta;
const additionalStatements: o.Statement[] = [];
@ -93,7 +97,8 @@ export function compileNgModule(meta: R3NgModuleMetadata): R3NgModuleDef {
declarations: o.Expression,
imports: o.Expression,
exports: o.Expression,
schemas: o.LiteralArrayExpr
schemas: o.LiteralArrayExpr,
id: o.Expression
};
// Only generate the keys in the metadata if the arrays have values.
@ -130,6 +135,10 @@ export function compileNgModule(meta: R3NgModuleMetadata): R3NgModuleDef {
definitionMap.schemas = o.literalArr(schemas.map(ref => ref.value));
}
if (id) {
definitionMap.id = id;
}
const expression = o.importExpr(R3.defineNgModule).callFn([mapToMapExpression(definitionMap)]);
const type = new o.ExpressionType(o.importExpr(R3.NgModuleDefWithMeta, [
new o.ExpressionType(moduleType), tupleTypeOf(declarations), tupleTypeOf(imports),

View File

@ -7,5 +7,5 @@
*/
export {CodegenComponentFactoryResolver as ɵCodegenComponentFactoryResolver} from './linker/component_factory_resolver';
export {registerModuleFactory as ɵregisterModuleFactory} from './linker/ng_module_factory_loader';
export {registerModuleFactory as ɵregisterModuleFactory} from './linker/ng_module_factory_registration';
export {ArgumentType as ɵArgumentType, BindingFlags as ɵBindingFlags, DepFlags as ɵDepFlags, EMPTY_ARRAY as ɵEMPTY_ARRAY, EMPTY_MAP as ɵEMPTY_MAP, NodeFlags as ɵNodeFlags, QueryBindingType as ɵQueryBindingType, QueryValueType as ɵQueryValueType, ViewDefinition as ɵViewDefinition, ViewFlags as ɵViewFlags, anchorDef as ɵand, createComponentFactory as ɵccf, createNgModuleFactory as ɵcmf, createRendererType2 as ɵcrt, directiveDef as ɵdid, elementDef as ɵeld, getComponentViewDefinitionFactory as ɵgetComponentViewDefinitionFactory, inlineInterpolate as ɵinlineInterpolate, interpolate as ɵinterpolate, moduleDef as ɵmod, moduleProvideDef as ɵmpd, ngContentDef as ɵncd, nodeValue as ɵnov, pipeDef as ɵpid, providerDef as ɵprd, pureArrayDef as ɵpad, pureObjectDef as ɵpod, purePipeDef as ɵppd, queryDef as ɵqud, textDef as ɵted, unwrapValue as ɵunv, viewDef as ɵvid} from './view/index';

View File

@ -106,6 +106,7 @@ export interface R3NgModuleMetadataFacade {
exports: Function[];
emitInline: boolean;
schemas: {name: string}[]|null;
id: string|null;
}
export interface R3InjectorMetadataFacade {

View File

@ -274,10 +274,9 @@ export {
SWITCH_RENDERER2_FACTORY__POST_R3__ as ɵSWITCH_RENDERER2_FACTORY__POST_R3__,
} from './render/api';
export {
getModuleFactory__POST_R3__ as ɵgetModuleFactory__POST_R3__,
registerNgModuleType as ɵregisterNgModuleType,
} from './linker/ng_module_factory_loader';
export { getModuleFactory__POST_R3__ as ɵgetModuleFactory__POST_R3__ } from './linker/ng_module_factory_loader';
export { registerNgModuleType as ɵregisterNgModuleType } from './linker/ng_module_factory_registration';
export {
publishGlobalUtil as ɵpublishGlobalUtil,

View File

@ -6,11 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Type} from '../interface/type';
import {NgModuleFactory as R3NgModuleFactory, NgModuleType} from '../render3/ng_module_ref';
import {stringify} from '../util/stringify';
import {NgModuleFactory} from './ng_module_factory';
import {getRegisteredNgModuleType} from './ng_module_factory_registration';
/**
@ -24,48 +23,14 @@ export abstract class NgModuleFactoryLoader {
abstract load(path: string): Promise<NgModuleFactory<any>>;
}
/**
* Map of module-id to the corresponding NgModule.
* - In pre Ivy we track NgModuleFactory,
* - In post Ivy we track the NgModuleType
*/
const modules = new Map<string, NgModuleFactory<any>|NgModuleType>();
/**
* Registers a loaded module. Should only be called from generated NgModuleFactory code.
* @publicApi
*/
export function registerModuleFactory(id: string, factory: NgModuleFactory<any>) {
const existing = modules.get(id) as NgModuleFactory<any>;
assertSameOrNotExisting(id, existing && existing.moduleType, factory.moduleType);
modules.set(id, factory);
}
function assertSameOrNotExisting(id: string, type: Type<any>| null, incoming: Type<any>): void {
if (type && type !== incoming) {
throw new Error(
`Duplicate module registered for ${id} - ${stringify(type)} vs ${stringify(type.name)}`);
}
}
export function registerNgModuleType(id: string, ngModuleType: NgModuleType) {
const existing = modules.get(id) as NgModuleType | null;
assertSameOrNotExisting(id, existing, ngModuleType);
modules.set(id, ngModuleType);
}
export function clearModulesForTest(): void {
modules.clear();
}
export function getModuleFactory__PRE_R3__(id: string): NgModuleFactory<any> {
const factory = modules.get(id) as NgModuleFactory<any>| null;
const factory = getRegisteredNgModuleType(id) as NgModuleFactory<any>| null;
if (!factory) throw noModuleError(id);
return factory;
}
export function getModuleFactory__POST_R3__(id: string): NgModuleFactory<any> {
const type = modules.get(id) as NgModuleType | null;
const type = getRegisteredNgModuleType(id) as NgModuleType | null;
if (!type) throw noModuleError(id);
return new R3NgModuleFactory(type);
}

View File

@ -0,0 +1,52 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Type} from '../interface/type';
import {NgModuleType} from '../render3/ng_module_ref';
import {stringify} from '../util/stringify';
import {NgModuleFactory} from './ng_module_factory';
/**
* Map of module-id to the corresponding NgModule.
* - In pre Ivy we track NgModuleFactory,
* - In post Ivy we track the NgModuleType
*/
const modules = new Map<string, NgModuleFactory<any>|NgModuleType>();
/**
* Registers a loaded module. Should only be called from generated NgModuleFactory code.
* @publicApi
*/
export function registerModuleFactory(id: string, factory: NgModuleFactory<any>) {
const existing = modules.get(id) as NgModuleFactory<any>;
assertSameOrNotExisting(id, existing && existing.moduleType, factory.moduleType);
modules.set(id, factory);
}
function assertSameOrNotExisting(id: string, type: Type<any>| null, incoming: Type<any>): void {
if (type && type !== incoming) {
throw new Error(
`Duplicate module registered for ${id} - ${stringify(type)} vs ${stringify(type.name)}`);
}
}
export function registerNgModuleType(id: string, ngModuleType: NgModuleType) {
const existing = modules.get(id) as NgModuleType | null;
assertSameOrNotExisting(id, existing, ngModuleType);
modules.set(id, ngModuleType);
}
export function clearModulesForTest(): void {
modules.clear();
}
export function getRegisteredNgModuleType(id: string) {
return modules.get(id);
}

View File

@ -75,6 +75,9 @@ export interface NgModuleDef<T> {
/** The set of schemas that declare elements to be allowed in the NgModule. */
schemas: SchemaMetadata[]|null;
/** Unique ID for the module with which it should be registered. */
id: string|null;
}
/**

View File

@ -366,6 +366,9 @@ export function ɵɵdefineNgModule<T>(def: {
/** The set of schemas that declare elements to be allowed in the NgModule. */
schemas?: SchemaMetadata[] | null;
/** Unique ID for the module that is used with `getModuleFactory`. */
id?: string | null;
}): never {
const res: NgModuleDef<T> = {
type: def.type,
@ -375,6 +378,7 @@ export function ɵɵdefineNgModule<T>(def: {
exports: def.exports || EMPTY_ARRAY,
transitiveCompileScopes: null,
schemas: def.schemas || null,
id: def.id || null,
};
return res as never;
}

View File

@ -9,7 +9,6 @@
import {ɵɵdefineInjectable, ɵɵdefineInjector,} from '../../di/interface/defs';
import {ɵɵinject} from '../../di/injector_compatibility';
import * as r3 from '../index';
import {registerNgModuleType} from '../../linker/ng_module_factory_loader';
import * as sanitization from '../../sanitization/sanitization';
@ -139,6 +138,4 @@ export const angularCoreEnv: {[name: string]: Function} = {
'ɵɵsanitizeScript': sanitization.ɵɵsanitizeScript,
'ɵɵsanitizeUrl': sanitization.ɵɵsanitizeUrl,
'ɵɵsanitizeUrlOrResourceUrl': sanitization.ɵɵsanitizeUrlOrResourceUrl,
'ɵregisterNgModuleType': registerNgModuleType,
};

View File

@ -11,7 +11,6 @@ import {resolveForwardRef} from '../../di/forward_ref';
import {NG_INJECTOR_DEF} from '../../di/interface/defs';
import {reflectDependencies} from '../../di/jit/util';
import {Type} from '../../interface/type';
import {registerNgModuleType} from '../../linker/ng_module_factory_loader';
import {Component} from '../../metadata';
import {ModuleWithProviders, NgModule, NgModuleDef, NgModuleTransitiveScopes} from '../../metadata/ng_module';
import {flatten} from '../../util/array_utils';
@ -117,14 +116,12 @@ export function compileNgModuleDefs(moduleType: NgModuleType, ngModule: NgModule
.map(expandModuleWithProviders),
emitInline: true,
schemas: ngModule.schemas ? flatten(ngModule.schemas) : null,
id: ngModule.id || null,
});
}
return ngModuleDef;
}
});
if (ngModule.id) {
registerNgModuleType(ngModule.id, moduleType);
}
let ngInjectorDef: any = null;
Object.defineProperty(moduleType, NG_INJECTOR_DEF, {

View File

@ -14,9 +14,11 @@ import {R3Injector, createInjector} from '../di/r3_injector';
import {Type} from '../interface/type';
import {ComponentFactoryResolver as viewEngine_ComponentFactoryResolver} from '../linker/component_factory_resolver';
import {InternalNgModuleRef, NgModuleFactory as viewEngine_NgModuleFactory, NgModuleRef as viewEngine_NgModuleRef} from '../linker/ng_module_factory';
import {registerNgModuleType} from '../linker/ng_module_factory_registration';
import {NgModuleDef} from '../metadata/ng_module';
import {assertDefined} from '../util/assert';
import {stringify} from '../util/stringify';
import {ComponentFactoryResolver} from './component_ref';
import {getNgModuleDef} from './definition';
import {maybeUnwrapFn} from './util/misc_utils';
@ -87,6 +89,11 @@ export class NgModuleFactory<T> extends viewEngine_NgModuleFactory<T> {
constructor(public moduleType: Type<T>) { super(); }
create(parentInjector: Injector|null): viewEngine_NgModuleRef<T> {
return new NgModuleRef(this.moduleType, parentInjector);
const moduleType = this.moduleType;
const moduleRef = new NgModuleRef(moduleType, parentInjector);
const ngModuleDef = getNgModuleDef(moduleType);
ngModuleDef && ngModuleDef.id &&
registerNgModuleType(ngModuleDef.id, moduleType as NgModuleType);
return moduleRef;
}
}

View File

@ -17,7 +17,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
import {modifiedInIvy, obsoleteInIvy, onlyInIvy} from '@angular/private/testing';
import {InternalNgModuleRef, NgModuleFactory} from '../../src/linker/ng_module_factory';
import {clearModulesForTest} from '../../src/linker/ng_module_factory_loader';
import {clearModulesForTest} from '../../src/linker/ng_module_factory_registration';
import {stringify} from '../../src/util/stringify';
class Engine {}
@ -327,6 +327,18 @@ function declareTests(config?: {useJit: boolean}) {
createModule(SomeOtherModule);
}).toThrowError(/Duplicate module registered/);
});
it('should not throw immediately if two modules have the same id', () => {
expect(() => {
@NgModule({id: 'some-module'})
class ModuleA {
}
@NgModule({id: 'some-module'})
class ModuleB {
}
}).not.toThrow();
});
});
describe('entryComponents', () => {

View File

@ -777,6 +777,7 @@ export declare function ɵɵdefineNgModule<T>(def: {
imports?: Type<any>[] | (() => Type<any>[]);
exports?: Type<any>[] | (() => Type<any>[]);
schemas?: SchemaMetadata[] | null;
id?: string | null;
}): never;
export declare function ɵɵdefinePipe<T>(pipeDef: {