Skip to content

Commit

Permalink
fix(compiler-cli): bypass static resolving of the component's changeD…
Browse files Browse the repository at this point in the history
…etection field in local compilation mode (#51848)

Currently the field changeDetection undergoes some static analysis to check if it is `ChangeDetectionStrategy` enum. Such static check fails in local compilation mode in g3 as the symbol cannot be resolved. So in local compilation mode we bypass such resolving and just write the expression as is into the component definition.

PR Close #51848
  • Loading branch information
pmvald authored and dylhunn committed Sep 26, 2023
1 parent dcaad16 commit 377a7ab
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 13 deletions.
Expand Up @@ -235,8 +235,14 @@ export class ComponentDecoratorHandler implements
const encapsulation: number =
resolveEnumValue(this.evaluator, component, 'encapsulation', 'ViewEncapsulation') ??
ViewEncapsulation.Emulated;
const changeDetection: number|null =
resolveEnumValue(this.evaluator, component, 'changeDetection', 'ChangeDetectionStrategy');

let changeDetection: number|Expression|null = null;
if (this.compilationMode !== CompilationMode.LOCAL) {
changeDetection =
resolveEnumValue(this.evaluator, component, 'changeDetection', 'ChangeDetectionStrategy');
} else if (component.has('changeDetection')) {
changeDetection = new WrappedNodeExpr(component.get('changeDetection')!);
}

let animations: Expression|null = null;
let animationTriggerNames: AnimationTriggerNames|null = null;
Expand Down Expand Up @@ -459,6 +465,7 @@ export class ComponentDecoratorHandler implements
ngContentSelectors: template.ngContentSelectors,
},
encapsulation,
changeDetection,
interpolation: template.interpolationConfig ?? DEFAULT_INTERPOLATION_CONFIG,
styles,

Expand Down Expand Up @@ -494,9 +501,7 @@ export class ComponentDecoratorHandler implements
},
diagnostics,
};
if (changeDetection !== null) {
output.analysis!.meta.changeDetection = changeDetection;
}

return output;
}

Expand Down
21 changes: 21 additions & 0 deletions packages/compiler-cli/test/ngtsc/local_compilation_spec.ts
Expand Up @@ -345,6 +345,27 @@ runInEachFileSystem(() => {
});
});

describe('component fields', () => {
it('should place the changeDetection as it is into the component def', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
import {SomeWeirdThing} from 'some-where';
@Component({
changeDetection: SomeWeirdThing,
template: '<span>Hello world!</span>',
})
export class MainComponent {
}
`);

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

expect(jsContents).toContain('changeDetection: SomeWeirdThing');
});
});

describe('constructor injection', () => {
it('should include injector types with all possible import/injection styles into component factory',
() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/jit_compiler_facade.ts
Expand Up @@ -208,7 +208,7 @@ export class CompilerFacadeImpl implements CompilerFacade {
styles: [...facade.styles, ...template.styles],
encapsulation: facade.encapsulation,
interpolation,
changeDetection: facade.changeDetection,
changeDetection: facade.changeDetection ?? null,
animations: facade.animations != null ? new WrappedNodeExpr(facade.animations) : null,
viewProviders: facade.viewProviders != null ? new WrappedNodeExpr(facade.viewProviders) :
null,
Expand Down
6 changes: 5 additions & 1 deletion packages/compiler/src/render3/partial/component.ts
Expand Up @@ -82,7 +82,11 @@ export function createComponentDefinitionMap(
definitionMap.set('viewProviders', meta.viewProviders);
definitionMap.set('animations', meta.animations);

if (meta.changeDetection !== undefined) {
if (meta.changeDetection !== null) {
if (typeof meta.changeDetection === 'object') {
throw new Error('Impossible state! Change detection flag is not resolved!');
}

definitionMap.set(
'changeDetection',
o.importExpr(R3.ChangeDetectionStrategy)
Expand Down
6 changes: 5 additions & 1 deletion packages/compiler/src/render3/view/api.ts
Expand Up @@ -292,8 +292,12 @@ export interface R3ComponentMetadata<DeclarationT extends R3TemplateDependency>

/**
* Strategy used for detecting changes in the component.
*
* In global compilation mode the value is ChangeDetectionStrategy if available as it is
* statically resolved during analysis phase. Whereas in local compilation mode the value is the
* expression as appears in the decorator.
*/
changeDetection?: ChangeDetectionStrategy;
changeDetection: ChangeDetectionStrategy|o.Expression|null;

/**
* The imports expression as appears on the component decorate for standalone component. This
Expand Down
16 changes: 11 additions & 5 deletions packages/compiler/src/render3/view/compiler.ts
Expand Up @@ -185,8 +185,6 @@ export function compileComponentFromMetadata(
const templateTypeName = meta.name;
const templateName = templateTypeName ? `${templateTypeName}_Template` : null;

const changeDetection = meta.changeDetection;

// Template compilation is currently conditional as we're in the process of rewriting it.
if (!USE_TEMPLATE_PIPELINE) {
// This is the main path currently used in compilation, which compiles the template with the
Expand Down Expand Up @@ -313,9 +311,17 @@ export function compileComponentFromMetadata(
'data', o.literalMap([{key: 'animation', value: meta.animations, quoted: false}]));
}

// Only set the change detection flag if it's defined and it's not the default.
if (changeDetection != null && changeDetection !== core.ChangeDetectionStrategy.Default) {
definitionMap.set('changeDetection', o.literal(changeDetection));
// Setting change detection flag
if (meta.changeDetection !== null) {
if (typeof meta.changeDetection === 'number' &&
meta.changeDetection !== core.ChangeDetectionStrategy.Default) {
// changeDetection is resolved during analysis. Only set it if not the default.
definitionMap.set('changeDetection', o.literal(meta.changeDetection));
} else if (typeof meta.changeDetection === 'object') {
// changeDetection is not resolved during analysis (e.g., we are in local compilation mode).
// So place it as is.
definitionMap.set('changeDetection', meta.changeDetection);
}
}

const expression =
Expand Down

0 comments on commit 377a7ab

Please sign in to comment.