Skip to content

Commit

Permalink
fix(compiler-cli): generating extra imports in local compilation mode…
Browse files Browse the repository at this point in the history
… when cycle is introduced (#53543)

At the moment the extra import generation in local compilation mode fails if these extra imports produce a cycle. To handle this, the cycle handling strategy is updated for local compilation, and following the behaviour in the full compilation mode, the compiler does not generate extra import if it leads to cycle and instead leave things to the runtime.

PR Close #53543
  • Loading branch information
pmvald authored and thePunderWoman committed Jan 30, 2024
1 parent 7e861c6 commit 64fa571
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 4 deletions.
Expand Up @@ -788,6 +788,11 @@ export class NgModuleDecoratorHandler implements
ngModuleStatements: Statement[], node: ClassDeclaration,
declarations: Reference<ClassDeclaration>[],
remoteScopesMayRequireCycleProtection: boolean): void {
// Local compilation mode generates its own runtimes to compute the dependencies. So there no
// need to add remote scope statements (which also conflicts with local compilation runtimes)
if (this.compilationMode === CompilationMode.LOCAL) {
return;
}
const context = getSourceFile(node);
for (const decl of declarations) {
const remoteScope = this.scopeRegistry.getRemoteScope(decl.node);
Expand Down
8 changes: 4 additions & 4 deletions packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Expand Up @@ -1100,12 +1100,12 @@ export class NgCompiler {
localCompilationExtraImportsTracker = new LocalCompilationExtraImportsTracker(checker);
}

// Cycles are handled in full compilation mode by "remote scoping".
// Cycles are handled in full and local compilation modes by "remote scoping".
// "Remote scoping" does not work well with tree shaking for libraries.
// So in partial compilation mode, when building a library, a cycle will cause an error.
const cycleHandlingStrategy = compilationMode === CompilationMode.FULL ?
CycleHandlingStrategy.UseRemoteScoping :
CycleHandlingStrategy.Error;
const cycleHandlingStrategy = compilationMode === CompilationMode.PARTIAL ?
CycleHandlingStrategy.Error :
CycleHandlingStrategy.UseRemoteScoping;

const strictCtorDeps = this.options.strictInjectionParameters || false;
const supportJitMode = this.options['supportJitMode'] ?? true;
Expand Down
43 changes: 43 additions & 0 deletions packages/compiler-cli/test/ngtsc/local_compilation_spec.ts
Expand Up @@ -262,6 +262,49 @@ runInEachFileSystem(
expect(env.getContents('main_comp.js')).toContain('import "internal_dir"');
expect(env.getContents('main_comp.js')).toContain('import "internal_pipe"');
});

it('should not include extra import and remote scope runtime for the local component dependencies when cycle is produced',
() => {
env.write('internal_comp.ts', `
import {Component} from '@angular/core';
import {cycleCreatingDep} from './main_comp';
@Component({template: '...', selector: 'internal-comp'})
export class InternalComp {
}
`);
env.write('internal_module.ts', `
import {NgModule} from '@angular/core';
import {InternalComp} from 'internal_comp';
@NgModule({declarations: [InternalComp], exports: [InternalComp]})
export class InternalModule {
}
`);
env.write('main_comp.ts', `
import {Component} from '@angular/core';
@Component({template: '<internal-comp></internal-comp>'})
export class MainComp {
}
`);
env.write('main_module.ts', `
import {NgModule} from '@angular/core';
import {MainComp} from 'main_comp';
import {InternalModule} from 'internal_module';
@NgModule({declarations: [MainComp], imports: [InternalModule]})
export class MainModule {
}
`);

env.driveMain();

expect(env.getContents('main_comp.js')).not.toContain('import "internal_comp"');
expect(env.getContents('main_module.js')).not.toContain('ɵɵsetComponentScope');
});
});

describe('ng module injector def', () => {
Expand Down

0 comments on commit 64fa571

Please sign in to comment.