Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix scope and injector issues in local compilation mode #51819

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -468,10 +468,7 @@ export class ComponentDecoratorHandler implements
viewProviders: wrappedViewProviders,
i18nUseExternalIds: this.i18nUseExternalIds,
relativeContextFilePath,
rawImports: (rawImports !== null && ts.isArrayLiteralExpression(rawImports) &&
rawImports.elements.length > 0) ?
new WrappedNodeExpr(rawImports) :
undefined,
rawImports: rawImports !== null ? new WrappedNodeExpr(rawImports) : undefined,
},
typeCheckMeta: extractDirectiveTypeCheckMeta(node, inputs, this.reflector),
classMetadata: this.includeClassMetadata ?
Expand Down
Expand Up @@ -445,12 +445,22 @@ export class NgModuleDecoratorHandler implements
};

if (this.compilationMode === CompilationMode.LOCAL) {
if (rawImports !== null && ts.isArrayLiteralExpression(rawImports) && rawImports.elements) {
injectorMetadata.imports.push(...rawImports.elements.map(n => new WrappedNodeExpr(n)));
}
// Adding NgModule's raw imports/exports to the injector's imports field in local compilation
// mode.
for (const exp of [rawImports, rawExports]) {
if (exp === null) {
continue;
}

if (rawExports !== null && ts.isArrayLiteralExpression(rawExports) && rawExports.elements) {
injectorMetadata.imports.push(...rawExports.elements.map(n => new WrappedNodeExpr(n)));
if (ts.isArrayLiteralExpression(exp)) {
// If array expression then add it entry-by-entry to the injector imports
if (exp.elements) {
injectorMetadata.imports.push(...exp.elements.map(n => new WrappedNodeExpr(n)));
}
} else {
// if not array expression then add it as is to the injector's imports field.
injectorMetadata.imports.push(new WrappedNodeExpr(exp));
}
}
}

Expand Down
118 changes: 113 additions & 5 deletions packages/compiler-cli/test/ngtsc/local_compilation_spec.ts
Expand Up @@ -62,7 +62,7 @@ runInEachFileSystem(() => {
expect(jsContents).toContain('MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({})');
});

it('should include raw module imports (including forward refs) in the injector def imports',
it('should include raw module imports array elements (including forward refs) in the injector def imports',
() => {
env.write('test.ts', `
import {NgModule, forwardRef} from '@angular/core';
Expand Down Expand Up @@ -90,7 +90,36 @@ runInEachFileSystem(() => {
'MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [SubModule1, forwardRef(() => SubModule2), LocalModule1, forwardRef(() => LocalModule2)] })');
});

it('should include raw module exports (including forward refs) in the injector def imports',
it('should include non-array raw module imports as it is in the injector def imports', () => {
env.write('test.ts', `
import {NgModule, forwardRef} from '@angular/core';
import {SubModule1} from './some-where';
import {SubModule2} from './another-where';

const NG_IMPORTS = [SubModule1, forwardRef(() => SubModule2), LocalModule1, forwardRef(() => LocalModule2)];

@NgModule({})
class LocalModule1 {}

@NgModule({
imports: NG_IMPORTS,
})
export class MainModule {
}

@NgModule({})
class LocalModule2 {}
`);

env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents)
.toContain(
'MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [NG_IMPORTS] })');
});

it('should include raw module exports array elements (including forward refs) in the injector def imports',
() => {
env.write('test.ts', `
import {NgModule, forwardRef} from '@angular/core';
Expand Down Expand Up @@ -118,7 +147,37 @@ runInEachFileSystem(() => {
'MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [SubModule1, forwardRef(() => SubModule2), LocalModule1, forwardRef(() => LocalModule2)] })');
});

it('should combine raw module imports and exports (including forward refs) in the injector def imports',
it('should include non-array raw module exports (including forward refs) in the injector def imports',
() => {
env.write('test.ts', `
import {NgModule, forwardRef} from '@angular/core';
import {SubModule1} from './some-where';
import {SubModule2} from './another-where';

const NG_EXPORTS = [SubModule1, forwardRef(() => SubModule2), LocalModule1, forwardRef(() => LocalModule2)];

@NgModule({})
class LocalModule1 {}

@NgModule({
exports: NG_EXPORTS,
})
export class MainModule {
}

@NgModule({})
class LocalModule2 {}
`);

env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents)
.toContain(
'MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [NG_EXPORTS] })');
});

it('should concat raw module imports and exports arrays (including forward refs) in the injector def imports',
() => {
env.write('test.ts', `
import {NgModule, forwardRef} from '@angular/core';
Expand All @@ -140,6 +199,29 @@ runInEachFileSystem(() => {
.toContain(
'MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [SubModule1, forwardRef(() => SubModule2), SubModule3, forwardRef(() => SubModule4)] })');
});

it('should combines non-array raw module imports and exports (including forward refs) in the injector def imports',
() => {
env.write('test.ts', `
import {NgModule, forwardRef} from '@angular/core';
import {NG_IMPORTS} from './some-where';
import {NG_EXPORTS} from './another-where';

@NgModule({
imports: NG_IMPORTS,
exports: NG_EXPORTS,
})
export class MainModule {
}
`);

env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents)
.toContain(
'MainModule.ɵinj = /*@__PURE__*/ i0.ɵɵdefineInjector({ imports: [NG_IMPORTS, NG_EXPORTS] })');
});
});

describe('component dependencies', () => {
Expand Down Expand Up @@ -194,7 +276,33 @@ runInEachFileSystem(() => {
'dependencies: i0.ɵɵgetComponentDepsFactory(MainComponent, [SomeThing, forwardRef(() => SomeThing2)])');
});

it('should not generate ɵɵgetComponentDepsFactory for standalone component with empty imports',
it('should generate ɵɵgetComponentDepsFactory with raw non-array imports as second param for component def dependencies - for standalone component with non-empty imports',
() => {
env.write('test.ts', `
import {Component, forwardRef} from '@angular/core';
import {SomeThing} from 'some-where';
import {SomeThing2} from 'some-where2';

const NG_IMPORTS = [SomeThing, forwardRef(()=>SomeThing2)];

@Component({
standalone: true,
imports: NG_IMPORTS,
selector: 'test-main',
template: '<span>Hello world!</span>',
})
export class MainComponent {
}
`);

env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents)
.toContain('dependencies: i0.ɵɵgetComponentDepsFactory(MainComponent, NG_IMPORTS)');
});

it('should generate ɵɵgetComponentDepsFactory with empty array as secon d arg for standalone component with empty imports',
() => {
env.write('test.ts', `
import {Component} from '@angular/core';
Expand All @@ -213,7 +321,7 @@ runInEachFileSystem(() => {
const jsContents = env.getContents('test.js');

expect(jsContents)
.toContain('dependencies: i0.ɵɵgetComponentDepsFactory(MainComponent)');
.toContain('dependencies: i0.ɵɵgetComponentDepsFactory(MainComponent, [])');
});

it('should not generate ɵɵgetComponentDepsFactory for standalone component with no imports',
Expand Down
12 changes: 11 additions & 1 deletion packages/core/src/render3/deps_tracker/api.ts
Expand Up @@ -29,6 +29,16 @@ interface ScopeData {
isPoisoned?: boolean;
}

/**
* Represents scope data for standalone components as calculated during runtime by the deps
* tracker.
*/
interface StandaloneCompScopeData extends ScopeData {
// Standalone components include the imported NgModules in their dependencies in order to
// determine their injector info. The following field stores the set of such NgModules.
ngModules: Set<NgModuleType<any>>;
pmvald marked this conversation as resolved.
Show resolved Hide resolved
}

/** Represents scope data for NgModule as calculated during runtime by the deps tracker. */
export interface NgModuleScope {
compilation: ScopeData;
Expand All @@ -39,7 +49,7 @@ export interface NgModuleScope {
* Represents scope data for standalone component as calculated during runtime by the deps tracker.
*/
export interface StandaloneComponentScope {
compilation: ScopeData;
compilation: StandaloneCompScopeData;
}

/** Component dependencies info as calculated during runtime by the deps tracker. */
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/render3/deps_tracker/deps_tracker.ts
Expand Up @@ -82,6 +82,7 @@ class DepsTracker implements DepsTrackerApi {
dependencies: [
...scope.compilation.directives,
...scope.compilation.pipes,
...scope.compilation.ngModules,
]
};
} else {
Expand Down Expand Up @@ -246,6 +247,7 @@ class DepsTracker implements DepsTrackerApi {
// Standalone components are always able to self-reference.
directives: new Set([type]),
pipes: new Set(),
ngModules: new Set(),
},
};

Expand All @@ -261,6 +263,7 @@ class DepsTracker implements DepsTrackerApi {
}

if (isNgModule(imported)) {
ans.compilation.ngModules.add(imported);
const importedScope = this.getNgModuleScope(imported);

// Short-circuit if an imported NgModule has corrupted exported scope.
Expand Down
27 changes: 21 additions & 6 deletions packages/core/test/render3/deps_tracker_spec.ts
Expand Up @@ -701,6 +701,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([]),
directives: new Set([MainComponent]),
ngModules: new Set([]),
});
});

Expand All @@ -726,6 +727,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([]),
});
});

Expand All @@ -751,6 +753,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([]),
});
});

Expand All @@ -767,7 +770,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation.isPoisoned).toBeTrue();
});

it('should include the exported scope of an imported module in the compilation scope', () => {
it('should include the imported module and its exported scope in the compilation scope', () => {
@Directive({})
class Directive1 {
}
Expand Down Expand Up @@ -799,10 +802,11 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([SubSubModule]),
});
});

it('should include the exported scope of an imported module in the compilation scope - case of nested array imports',
it('should include the imported module and its exported scope in the compilation scope - case of nested array imports',
() => {
@Directive({})
class Directive1 {
Expand Down Expand Up @@ -835,6 +839,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([SubSubModule]),
});
});

Expand Down Expand Up @@ -878,6 +883,7 @@ describe('runtime dependency tracker', () => {
pipes: new Set([Pipe1, SubModulePipe]),
directives: new Set(
[MainComponent, Component1, Directive1, SubModuleComponent, SubModuleDirective]),
ngModules: new Set([SubModule]),
});
});

Expand Down Expand Up @@ -922,6 +928,7 @@ describe('runtime dependency tracker', () => {
pipes: new Set([Pipe1, SubModulePipe]),
directives: new Set(
[MainComponent, Component1, Directive1, SubModuleComponent, SubModuleDirective]),
ngModules: new Set([SubModule]),
});
});

Expand All @@ -946,13 +953,15 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([]),
});

ans = depsTracker.getStandaloneComponentScope(MainComponent as ComponentType<any>, []);

expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([]),
});
});

Expand All @@ -979,6 +988,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([Pipe1]),
directives: new Set([MainComponent, Component1, Directive1]),
ngModules: new Set([]),
});

depsTracker.clearScopeCacheFor(MainComponent as ComponentType<any>);
Expand All @@ -987,6 +997,7 @@ describe('runtime dependency tracker', () => {
expect(ans.compilation).toEqual({
pipes: new Set([]),
directives: new Set([MainComponent]),
ngModules: new Set([]),
});
});
});
Expand Down Expand Up @@ -1172,7 +1183,7 @@ describe('runtime dependency tracker', () => {
expect(ans.dependencies).toEqual([]);
});

it('should include the exported scope of imported module', () => {
it('should include the imported module and its exported scope', () => {
@Component({})
class Component1 {
}
Expand All @@ -1199,11 +1210,15 @@ describe('runtime dependency tracker', () => {
depsTracker.getComponentDependencies(MainComponent as ComponentType<any>, [SubModule]);

expect(ans.dependencies).toEqual(jasmine.arrayWithExactContents([
MainComponent, Component1, Directive1, Pipe1
MainComponent,
Component1,
Directive1,
Pipe1,
SubModule,
]));
});

it('should include the exported scope of imported forward ref module', () => {
it('should include the imported forward ref module and its exported scope', () => {
@Component({})
class Component1 {
}
Expand All @@ -1230,7 +1245,7 @@ describe('runtime dependency tracker', () => {
MainComponent as ComponentType<any>, [forwardRef(() => SubModule)]);

expect(ans.dependencies).toEqual(jasmine.arrayWithExactContents([
MainComponent, Component1, Directive1, Pipe1
MainComponent, Component1, Directive1, Pipe1, SubModule
]));
});

Expand Down