Skip to content

Commit 3e45f7b

Browse files
crisbetocrisbeto
committed
fix(compiler-cli): correctly attach pure comments
Fixes that we weren't accounting for the Closure-specific pure comments. Also fixes a bug with how we were attaching the comments.
1 parent 8374641 commit 3e45f7b

5 files changed

Lines changed: 120 additions & 85 deletions

File tree

‎packages/compiler-cli/preprocessor/src/processor.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2762,17 +2762,35 @@ function createStaticField(
27622762
pure: boolean,
27632763
isClosureCompilerEnabled?: boolean,
27642764
): string {
2765-
const pureComment = pure ? '/*@__PURE__*/ ' : '';
27662765
const typeSuffix = type ? `: ${type}` : '';
27672766
const nocollapse = isClosureCompilerEnabled ? '/** @nocollapse */\n ' : '';
2767+
const tsignore = '// @ts-ignore';
27682768
// `// @ts-ignore` only covers the line directly below it, so the initializer has to start on the
27692769
// `static <name> =` line. Breaking after `=` would leave the guard covering the declaration
27702770
// instead of the value it was emitted for. ngtsc emits this on one line too.
27712771
//
27722772
// TODO(parity): `// @ts-ignore` only covers the initializer's first line, so properties emitted
27732773
// after multi-line `hostBindings` or `template` functions land on uncovered continuation lines
27742774
// (see `tests/emit_suppressions.test.ts`).
2775-
return ` ${nocollapse}// @ts-ignore\n static ${name}${typeSuffix} = ${pureComment}${value};\n`;
2775+
if (!pure) {
2776+
return ` ${nocollapse}${tsignore}\n static ${name}${typeSuffix} = ${value};\n`;
2777+
}
2778+
if (!isClosureCompilerEnabled) {
2779+
return ` ${nocollapse}${tsignore}\n static ${name}${typeSuffix} = /*@__PURE__*/ ${value};\n`;
2780+
}
2781+
// ngtsc attaches the Closure annotation as a synthetic comment during its own emit, so it
2782+
// always lands right in front of the call. This output is compiled by a downstream `tsc`,
2783+
// whose emitter drops a comment sitting between `=` and the initializer whenever it rewrites
2784+
// the property (stripping the type annotation, or downleveling the static field to
2785+
// `Class.ɵcmp = ...`), but preserves it as leading trivia of an initializer that starts on its
2786+
// own line. The initializer line gets its own `// @ts-ignore`, since the one above `static`
2787+
// no longer reaches it.
2788+
return (
2789+
` ${nocollapse}${tsignore}\n` +
2790+
` static ${name}${typeSuffix} =\n` +
2791+
` ${tsignore}\n` +
2792+
` /** @pureOrBreakMyCode */ ${value};\n`
2793+
);
27762794
}
27772795

27782796
function compileFactoryField(

‎packages/compiler-cli/preprocessor/test/golden/closure_compiler/golden.md‎

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,9 @@ export class MyService {
1818
};
1919
/** @nocollapse */
2020
// @ts-ignore
21-
static ɵprov: i0.ɵɵInjectableDeclaration<any> = /*@__PURE__*/ i0.ɵɵdefineInjectable({
22-
token: MyService,
23-
factory: MyService.ɵfac,
24-
});
21+
static ɵprov: i0.ɵɵInjectableDeclaration<any> =
22+
// @ts-ignore
23+
/** @pureOrBreakMyCode */ i0.ɵɵdefineInjectable({ token: MyService, factory: MyService.ɵfac });
2524
static {
2625
(typeof ngDevMode === 'undefined' || ngDevMode) &&
2726
i0.ɵsetClassMetadata(MyService, [{ type: Injectable }], null, null);
@@ -49,25 +48,27 @@ export class MyApp {
4948
never,
5049
true,
5150
never
52-
> = /*@__PURE__*/ i0.ɵɵdefineComponent({
53-
type: MyApp,
54-
selectors: [['my-app']],
55-
decls: 2,
56-
vars: 0,
57-
template: function MyApp_Template(rf: number, ctx: any): any {
58-
if (rf & 1) {
59-
i0.ɵɵelementStart(0, 'div');
60-
i0.ɵɵtext(
61-
1,
62-
'This is a very long template string designed to exceed the one hundred character threshold of the constant pool sharing mechanism in the Angular compiler when annotateForClosureCompiler is enabled.',
63-
);
64-
i0.ɵɵelementEnd();
65-
}
66-
},
67-
styles: [
68-
'div[_ngcontent-%COMP%] { color: red; --%NS%long-prop: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; }',
69-
],
70-
});
51+
> =
52+
// @ts-ignore
53+
/** @pureOrBreakMyCode */ i0.ɵɵdefineComponent({
54+
type: MyApp,
55+
selectors: [['my-app']],
56+
decls: 2,
57+
vars: 0,
58+
template: function MyApp_Template(rf: number, ctx: any): any {
59+
if (rf & 1) {
60+
i0.ɵɵelementStart(0, 'div');
61+
i0.ɵɵtext(
62+
1,
63+
'This is a very long template string designed to exceed the one hundred character threshold of the constant pool sharing mechanism in the Angular compiler when annotateForClosureCompiler is enabled.',
64+
);
65+
i0.ɵɵelementEnd();
66+
}
67+
},
68+
styles: [
69+
'div[_ngcontent-%COMP%] { color: red; --%NS%long-prop: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; }',
70+
],
71+
});
7172
static {
7273
(typeof ngDevMode === 'undefined' || ngDevMode) &&
7374
i0.ɵsetClassMetadata(
@@ -117,20 +118,22 @@ export class MyOtherApp {
117118
never,
118119
true,
119120
never
120-
> = /*@__PURE__*/ i0.ɵɵdefineComponent({
121-
type: MyOtherApp,
122-
selectors: [['my-other-app']],
123-
decls: 2,
124-
vars: 0,
125-
template: function MyOtherApp_Template(rf: number, ctx: any): any {
126-
if (rf & 1) {
127-
i0.ɵɵelementStart(0, 'div');
128-
i0.ɵɵtext(1, 'Short template');
129-
i0.ɵɵelementEnd();
130-
}
131-
},
132-
styles: [_c0()],
133-
});
121+
> =
122+
// @ts-ignore
123+
/** @pureOrBreakMyCode */ i0.ɵɵdefineComponent({
124+
type: MyOtherApp,
125+
selectors: [['my-other-app']],
126+
decls: 2,
127+
vars: 0,
128+
template: function MyOtherApp_Template(rf: number, ctx: any): any {
129+
if (rf & 1) {
130+
i0.ɵɵelementStart(0, 'div');
131+
i0.ɵɵtext(1, 'Short template');
132+
i0.ɵɵelementEnd();
133+
}
134+
},
135+
styles: [_c0()],
136+
});
134137
static {
135138
(typeof ngDevMode === 'undefined' || ngDevMode) &&
136139
i0.ɵsetClassMetadata(

‎packages/compiler-cli/preprocessor/test/golden/closure_compiler_bazel_options/golden.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ export class BazelService {
1717
};
1818
/** @nocollapse */
1919
// @ts-ignore
20-
static ɵprov: i0.ɵɵInjectableDeclaration<any> = /*@__PURE__*/ i0.ɵɵdefineService({
21-
token: BazelService,
22-
factory: BazelService.ɵfac,
23-
});
20+
static ɵprov: i0.ɵɵInjectableDeclaration<any> =
21+
// @ts-ignore
22+
/** @pureOrBreakMyCode */ i0.ɵɵdefineService({
23+
token: BazelService,
24+
factory: BazelService.ɵfac,
25+
});
2426
static {
2527
(typeof ngDevMode === 'undefined' || ngDevMode) &&
2628
i0.ɵsetClassMetadata(BazelService, [{ type: Service }], null, null);

‎packages/compiler-cli/preprocessor/test/golden/closure_compiler_static_nocollapse/golden.md‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,13 @@ export class HybridService {
3535
};
3636
/** @nocollapse */
3737
// @ts-ignore
38-
static ɵprov: i0.ɵɵInjectableDeclaration<any> = /*@__PURE__*/ i0.ɵɵdefineInjectable({
39-
token: HybridService,
40-
factory: HybridService.ɵfac,
41-
providedIn: 'root',
42-
});
38+
static ɵprov: i0.ɵɵInjectableDeclaration<any> =
39+
// @ts-ignore
40+
/** @pureOrBreakMyCode */ i0.ɵɵdefineInjectable({
41+
token: HybridService,
42+
factory: HybridService.ɵfac,
43+
providedIn: 'root',
44+
});
4345
static {
4446
(typeof ngDevMode === 'undefined' || ngDevMode) &&
4547
i0.ɵsetClassMetadata(
@@ -72,14 +74,16 @@ export class MyCmp {
7274
never,
7375
true,
7476
never
75-
> = /*@__PURE__*/ i0.ɵɵdefineComponent({
76-
type: MyCmp,
77-
selectors: [['my-cmp']],
78-
decls: 0,
79-
vars: 0,
80-
template: function MyCmp_Template(rf: number, ctx: any): any {},
81-
encapsulation: 2,
82-
});
77+
> =
78+
// @ts-ignore
79+
/** @pureOrBreakMyCode */ i0.ɵɵdefineComponent({
80+
type: MyCmp,
81+
selectors: [['my-cmp']],
82+
decls: 0,
83+
vars: 0,
84+
template: function MyCmp_Template(rf: number, ctx: any): any {},
85+
encapsulation: 2,
86+
});
8387
static {
8488
(typeof ngDevMode === 'undefined' || ngDevMode) &&
8589
i0.ɵsetClassMetadata(
@@ -119,11 +123,9 @@ export class MyPipe implements PipeTransform {
119123
};
120124
/** @nocollapse */
121125
// @ts-ignore
122-
static ɵpipe: i0.ɵɵPipeDeclaration<MyPipe, 'myPipe', true> = /*@__PURE__*/ i0.ɵɵdefinePipe({
123-
name: 'myPipe',
124-
type: MyPipe,
125-
pure: true,
126-
});
126+
static ɵpipe: i0.ɵɵPipeDeclaration<MyPipe, 'myPipe', true> =
127+
// @ts-ignore
128+
/** @pureOrBreakMyCode */ i0.ɵɵdefinePipe({ name: 'myPipe', type: MyPipe, pure: true });
127129
static {
128130
(typeof ngDevMode === 'undefined' || ngDevMode) &&
129131
i0.ɵsetClassMetadata(MyPipe, [{ type: Pipe, args: [{ name: 'myPipe' }] }], null, null);
@@ -143,10 +145,12 @@ export class StillDecorated {
143145
};
144146
/** @nocollapse */
145147
// @ts-ignore
146-
static ɵprov: i0.ɵɵInjectableDeclaration<any> = /*@__PURE__*/ i0.ɵɵdefineInjectable({
147-
token: StillDecorated,
148-
factory: StillDecorated.ɵfac,
149-
});
148+
static ɵprov: i0.ɵɵInjectableDeclaration<any> =
149+
// @ts-ignore
150+
/** @pureOrBreakMyCode */ i0.ɵɵdefineInjectable({
151+
token: StillDecorated,
152+
factory: StillDecorated.ɵfac,
153+
});
150154
static {
151155
(typeof ngDevMode === 'undefined' || ngDevMode) &&
152156
i0.ɵsetClassMetadata(StillDecorated, [{ type: Injectable }], null, null);

‎packages/compiler-cli/src/ngtsc/testing/src/ngp_emit_normalizer.ts‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,10 @@ export function normalizeNgpEmitForComparison(code: string, ellipsisToken = '\u2
186186
/static\s+(ɵ(?:cmp|dir|pipe|mod|inj|prov|fac))\s*=\s*(?!\/\*\s*@__PURE__\s*\*\/)(i0\.ɵɵdefine|\(\(\)\s*=>)/g,
187187
'static $1 = /*@__PURE__*/ $2',
188188
)
189+
// Under `annotateForClosureCompiler` NGP puts `/** @pureOrBreakMyCode */` on the line after
190+
// `=` (the only placement `tsc` carries through to the downleveled assignment); ngtsc emits
191+
// it on the same line.
192+
.replace(/=\s*\n\s*(\/\*\* @pureOrBreakMyCode \*\/)/g, '= $1')
189193
.replace(
190194
/\(\(\(typeof\s+(ngDevMode|ngJitMode)\s*===\s*(['"])undefined\2\)\s*\|\|\s*\1\)/g,
191195
'((typeof $1 === $2undefined$2 || $1)',
@@ -207,24 +211,28 @@ export function normalizeNgpEmitForComparison(code: string, ellipsisToken = '\u2
207211
* `NgtscTestEnvironment.getContents()`.
208212
*/
209213
export function normalizeNgpOutputFile(content: string): string {
210-
return content
211-
.replace(/^[ \t]*\/\/[ \t]*@ts-ignore[^\n]*\r?\n/gm, '')
212-
.replace(/\/\*\s*@ts-ignore\s*\*\/\s*/g, '')
213-
.replace(
214-
/(\.(?:ɵcmp|ɵdir|ɵpipe|ɵmod|ɵinj|ɵprov)\s*=\s*)(?!\/\*\s*@__PURE__\s*\*\/)(i0\.ɵɵdefine)/g,
215-
'$1/*@__PURE__*/ $2',
216-
)
217-
.replace(
218-
/(\.ɵfac\s*=\s*function\s+[A-Za-z0-9_$]+\(__ngFactoryType__\)\s*\{)\s*\n\s*(return\s+new\s+\(__ngFactoryType__\s*\|\|\s*[A-Za-z0-9_$]+\)\([^;\n]*\);)\s*\n\s*\};/g,
219-
'$1 $2 };',
220-
)
221-
.replace(
222-
/\(\(\(typeof\s+(ngDevMode|ngJitMode)\s*===\s*(['"])undefined\2\)\s*\|\|\s*\1\)/g,
223-
'((typeof $1 === $2undefined$2 || $1)',
224-
)
225-
.replace(
226-
/\(\(typeof\s+(ngDevMode|ngJitMode)\s*===\s*(['"])undefined\2\s*\|\|\s*\1\)\s*&&\s*([^;\n]+)\);/g,
227-
'(typeof $1 === $2undefined$2 || $1) && $3;',
228-
)
229-
.replace(/'([^'\\\n]*)'/g, '"$1"');
214+
return (
215+
content
216+
.replace(/^[ \t]*\/\/[ \t]*@ts-ignore[^\n]*\r?\n/gm, '')
217+
.replace(/\/\*\s*@ts-ignore\s*\*\/\s*/g, '')
218+
.replace(
219+
/(\.(?:ɵcmp|ɵdir|ɵpipe|ɵmod|ɵinj|ɵprov)\s*=\s*)(?!\/\*\s*@__PURE__\s*\*\/)(i0\.ɵɵdefine)/g,
220+
'$1/*@__PURE__*/ $2',
221+
)
222+
// See `normalizeNgpEmitForComparison`: the Closure pure marker lands on the line after `=`.
223+
.replace(/=\s*\n\s*(\/\*\* @pureOrBreakMyCode \*\/)/g, '= $1')
224+
.replace(
225+
/(\.ɵfac\s*=\s*function\s+[A-Za-z0-9_$]+\(__ngFactoryType__\)\s*\{)\s*\n\s*(return\s+new\s+\(__ngFactoryType__\s*\|\|\s*[A-Za-z0-9_$]+\)\([^;\n]*\);)\s*\n\s*\};/g,
226+
'$1 $2 };',
227+
)
228+
.replace(
229+
/\(\(\(typeof\s+(ngDevMode|ngJitMode)\s*===\s*(['"])undefined\2\)\s*\|\|\s*\1\)/g,
230+
'((typeof $1 === $2undefined$2 || $1)',
231+
)
232+
.replace(
233+
/\(\(typeof\s+(ngDevMode|ngJitMode)\s*===\s*(['"])undefined\2\s*\|\|\s*\1\)\s*&&\s*([^;\n]+)\);/g,
234+
'(typeof $1 === $2undefined$2 || $1) && $3;',
235+
)
236+
.replace(/'([^'\\\n]*)'/g, '"$1"')
237+
);
230238
}

0 commit comments

Comments
 (0)