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

perf(core): avoid recursive scope recalculation when TestBed.overrideModule is used #35454

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
24 changes: 8 additions & 16 deletions packages/core/src/render3/jit/module.ts
Expand Up @@ -427,25 +427,19 @@ export function patchComponentDefWithScope<C>(
/**
* Compute the pair of transitive scopes (compilation scope and exported scope) for a given module.
*
* By default this operation is memoized and the result is cached on the module's definition. You
* can avoid memoization and previously stored results (if available) by providing the second
* argument with the `true` value (forcing transitive scopes recalculation).
*
* This function can be called on modules with components that have not fully compiled yet, but the
* result should not be used until they have.
* This operation is memoized and the result is cached on the module's definition. This function can
* be called on modules with components that have not fully compiled yet, but the result should not
* be used until they have.
*
* @param moduleType module that transitive scope should be calculated for.
* @param forceRecalc flag that indicates whether previously calculated and memoized values should
* be ignored and transitive scope to be fully recalculated.
*/
export function transitiveScopesFor<T>(
moduleType: Type<T>, forceRecalc: boolean = false): NgModuleTransitiveScopes {
export function transitiveScopesFor<T>(moduleType: Type<T>): NgModuleTransitiveScopes {
if (!isNgModule(moduleType)) {
throw new Error(`${moduleType.name} does not have a module def (ɵmod property)`);
}
const def = getNgModuleDef(moduleType) !;

if (!forceRecalc && def.transitiveCompileScopes !== null) {
if (def.transitiveCompileScopes !== null) {
return def.transitiveCompileScopes;
}

Expand Down Expand Up @@ -486,7 +480,7 @@ export function transitiveScopesFor<T>(

// When this module imports another, the imported module's exported directives and pipes are
// added to the compilation scope of this module.
const importedScope = transitiveScopesFor(importedType, forceRecalc);
const importedScope = transitiveScopesFor(importedType);
importedScope.exported.directives.forEach(entry => scopes.compilation.directives.add(entry));
importedScope.exported.pipes.forEach(entry => scopes.compilation.pipes.add(entry));
});
Expand All @@ -505,7 +499,7 @@ export function transitiveScopesFor<T>(
if (isNgModule(exportedType)) {
// When this module exports another, the exported module's exported directives and pipes are
// added to both the compilation and exported scopes of this module.
const exportedScope = transitiveScopesFor(exportedType, forceRecalc);
const exportedScope = transitiveScopesFor(exportedType);
exportedScope.exported.directives.forEach(entry => {
scopes.compilation.directives.add(entry);
scopes.exported.directives.add(entry);
Expand All @@ -521,9 +515,7 @@ export function transitiveScopesFor<T>(
}
});

if (!forceRecalc) {
def.transitiveCompileScopes = scopes;
}
def.transitiveCompileScopes = scopes;
return scopes;
}

Expand Down
20 changes: 14 additions & 6 deletions packages/core/test/test_bed_spec.ts
Expand Up @@ -889,15 +889,23 @@ describe('TestBed', () => {
{set: {template: `<span someDirective>{{'hello' | somePipe}}</span>`}});
TestBed.createComponent(SomeComponent);

const defBeforeReset = (SomeComponent as any).ɵcmp;
expect(defBeforeReset.pipeDefs().length).toEqual(1);
expect(defBeforeReset.directiveDefs().length).toEqual(2); // directive + component
const cmpDefBeforeReset = (SomeComponent as any).ɵcmp;
expect(cmpDefBeforeReset.pipeDefs().length).toEqual(1);
expect(cmpDefBeforeReset.directiveDefs().length).toEqual(2); // directive + component

const modDefBeforeReset = (SomeModule as any).ɵmod;
const transitiveScope = modDefBeforeReset.transitiveCompileScopes.compilation;
expect(transitiveScope.pipes.size).toEqual(1);
expect(transitiveScope.directives.size).toEqual(2);

TestBed.resetTestingModule();

const defAfterReset = (SomeComponent as any).ɵcmp;
expect(defAfterReset.pipeDefs).toBe(null);
expect(defAfterReset.directiveDefs).toBe(null);
const cmpDefAfterReset = (SomeComponent as any).ɵcmp;
expect(cmpDefAfterReset.pipeDefs).toBe(null);
expect(cmpDefAfterReset.directiveDefs).toBe(null);

const modDefAfterReset = (SomeModule as any).ɵmod;
expect(modDefAfterReset.transitiveCompileScopes).toBe(null);
});

it('should cleanup ng defs for classes with no ng annotations (in case of inheritance)',
Expand Down
7 changes: 5 additions & 2 deletions packages/core/testing/src/r3_test_bed_compiler.ts
Expand Up @@ -355,8 +355,11 @@ export class R3TestBedCompiler {
// are present, always re-calculate transitive scopes to have the most up-to-date
// information available. The `moduleToScope` map avoids repeated re-calculation of
// scopes for the same module.
const forceRecalc = !isTestingModule && this.hasModuleOverrides;
moduleToScope.set(moduleType, transitiveScopesFor(realType, forceRecalc));
if (!isTestingModule && this.hasModuleOverrides) {
this.storeFieldOfDefOnType(moduleType as any, NG_MOD_DEF, 'transitiveCompileScopes');
(moduleType as any)[NG_MOD_DEF].transitiveCompileScopes = null;
AndrewKushnir marked this conversation as resolved.
Show resolved Hide resolved
}
moduleToScope.set(moduleType, transitiveScopesFor(realType));
}
return moduleToScope.get(moduleType) !;
};
Expand Down