Skip to content

Commit 412788f

Browse files
crisbetomattrbeck
crisbeto
authored and
mattrbeck
committed
fix(compiler): ensure generated code compiles
Initial pass to make sure some common cases produce code that compiles.
1 parent ed9750d commit 412788f

File tree

23 files changed

+295
-51
lines changed

23 files changed

+295
-51
lines changed

‎packages/compiler-cli/linker/babel/src/ast/babel_ast_factory.ts‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,14 @@ export class BabelAstFactory implements AstFactory<
2828
t.Expression | t.SpreadElement,
2929
t.TSType
3030
> {
31+
private readonly typesEnabled: boolean;
32+
3133
constructor(
3234
/** The absolute path to the source file being compiled. */
33-
private sourceUrl: string,
34-
) {}
35+
private sourcePath: string,
36+
) {
37+
this.typesEnabled = sourcePath.endsWith('.ts') || sourcePath.endsWith('.mts');
38+
}
3539

3640
attachComments(statement: t.Statement | t.Expression, leadingComments: LeadingComment[]): void {
3741
// We must process the comments in reverse because `t.addComment()` will add new ones in front.
@@ -112,7 +116,7 @@ export class BabelAstFactory implements AstFactory<
112116
assert(body, t.isBlockStatement, 'a block');
113117
return t.functionDeclaration(
114118
t.identifier(functionName),
115-
parameters.map((param) => identifierWithType(param.name, param.type)),
119+
parameters.map((param) => this.identifierWithType(param.name, param.type)),
116120
body,
117121
);
118122
}
@@ -125,7 +129,7 @@ export class BabelAstFactory implements AstFactory<
125129
assert(body, t.isBlockStatement, 'a block');
126130
}
127131
return t.arrowFunctionExpression(
128-
parameters.map((param) => identifierWithType(param.name, param.type)),
132+
parameters.map((param) => this.identifierWithType(param.name, param.type)),
129133
body,
130134
);
131135
}
@@ -139,7 +143,7 @@ export class BabelAstFactory implements AstFactory<
139143
const name = functionName !== null ? t.identifier(functionName) : null;
140144
return t.functionExpression(
141145
name,
142-
parameters.map((param) => identifierWithType(param.name, param.type)),
146+
parameters.map((param) => this.identifierWithType(param.name, param.type)),
143147
body,
144148
);
145149
}
@@ -234,7 +238,7 @@ export class BabelAstFactory implements AstFactory<
234238
type: t.TSType | null,
235239
): t.Statement {
236240
return t.variableDeclaration(variableType, [
237-
t.variableDeclarator(identifierWithType(variableName, type), initializer),
241+
t.variableDeclarator(this.identifierWithType(variableName, type), initializer),
238242
]);
239243
}
240244

@@ -253,7 +257,7 @@ export class BabelAstFactory implements AstFactory<
253257
// Add in the filename so that we can map to external template files.
254258
// Note that Babel gets confused if you specify a filename when it is the original source
255259
// file. This happens when the template is inline, in which case just use `undefined`.
256-
filename: sourceMapRange.url !== this.sourceUrl ? sourceMapRange.url : undefined,
260+
filename: sourceMapRange.url !== this.sourcePath ? sourceMapRange.url : undefined,
257261
start: {
258262
line: sourceMapRange.start.line + 1, // lines are 1-based in Babel.
259263
column: sourceMapRange.start.column,
@@ -301,7 +305,7 @@ export class BabelAstFactory implements AstFactory<
301305
}
302306

303307
createMapType(valueType: t.TSType): t.TSType {
304-
const keySignature = identifierWithType('key', this.createBuiltInType('string'));
308+
const keySignature = this.identifierWithType('key', this.createBuiltInType('string'));
305309
return t.tsTypeLiteral([t.tsIndexSignature([keySignature], t.tsTypeAnnotation(valueType))]);
306310
}
307311

@@ -311,6 +315,16 @@ export class BabelAstFactory implements AstFactory<
311315
}
312316
throw new Error('Attempting to transplant a type node from a non-Babel AST: ' + type);
313317
}
318+
319+
private identifierWithType(name: string, type: t.TSType | null): t.Identifier {
320+
const node = t.identifier(name);
321+
322+
if (this.typesEnabled && type != null) {
323+
node.typeAnnotation = t.tsTypeAnnotation(type);
324+
}
325+
326+
return node;
327+
}
314328
}
315329

316330
function getEntityTypeFromExpression(expression: t.Expression): t.Identifier | t.TSQualifiedName {
@@ -333,16 +347,6 @@ function getEntityTypeFromExpression(expression: t.Expression): t.Identifier | t
333347
throw new Error(`Unsupported expression for type reference: ${expression.type}`);
334348
}
335349

336-
function identifierWithType(name: string, type: t.TSType | null): t.Identifier {
337-
const node = t.identifier(name);
338-
339-
if (type !== null) {
340-
node.typeAnnotation = t.tsTypeAnnotation(type);
341-
}
342-
343-
return node;
344-
}
345-
346350
function isLExpression(expr: t.Expression): expr is Extract<t.LVal, t.Expression> {
347351
// Some LVal types are not expressions, which prevents us from using `t.isLVal()`
348352
// directly with `assert()`.

‎packages/compiler-cli/linker/src/linker_import_generator.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ import {FatalLinkerError} from './fatal_linker_error';
1717
* must be achieved by property access on an `ng` namespace identifier, which is passed in via the
1818
* constructor.
1919
*/
20-
export class LinkerImportGenerator<TStatement, TExpression, TType>
21-
implements ImportGenerator<null, TExpression>
22-
{
20+
export class LinkerImportGenerator<TStatement, TExpression, TType> implements ImportGenerator<
21+
null,
22+
TExpression
23+
> {
2324
constructor(
2425
private factory: AstFactory<TStatement, TExpression, TType>,
2526
private ngImport: TExpression,

‎packages/compiler-cli/linker/test/ast/ast_value_spec.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,14 @@ describe('AstValue', () => {
390390

391391
describe('getFunctionParameters', () => {
392392
it('should return the parameters of a function expression', () => {
393-
const funcExpr = factory.createFunctionExpression('foo', ['a', 'b'], factory.createBlock([]));
393+
const funcExpr = factory.createFunctionExpression(
394+
'foo',
395+
[
396+
{name: 'a', type: null},
397+
{name: 'b', type: null},
398+
],
399+
factory.createBlock([]),
400+
);
394401
expect(createAstValue<Function>(funcExpr).getFunctionParameters()).toEqual(
395402
['a', 'b'].map((name) => createAstValue(factory.createIdentifier(name))),
396403
);

‎packages/compiler-cli/test/compliance/isolated/isolated_compile_spec.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import ts from 'typescript';
1010
import {checkExpectations} from '../test_helpers/check_expectations';
1111
import {checkNoUnexpectedErrors} from '../test_helpers/check_errors';
12-
import {FileSystem} from '../../../src/ngtsc/file_system';
12+
import {AbsoluteFsPath, FileSystem} from '../../../src/ngtsc/file_system';
1313
import {NgtscTestCompilerHost} from '../../../src/ngtsc/testing';
1414
import {
1515
getBuildOutputDirectory,
@@ -25,8 +25,8 @@ describe('isolated compliance tests', () => {
2525
if (!test.relativePath.includes('isolated')) {
2626
continue;
2727
}
28-
describe(`[${test.relativePath}]`, () => {
29-
it(test.description, () => {
28+
describe(`[${test.description}]`, () => {
29+
(test.focusTest ? fit : it)(test.description, () => {
3030
const fs = initMockTestFileSystem(test.realTestPath);
3131
const {errors} = compileTests(fs, test);
3232
for (const expectation of test.expectations) {
@@ -59,8 +59,8 @@ function compileTests(fs: FileSystem, test: ComplianceTest): {errors: string[]}
5959

6060
const transformedFiles = preprocessor.transformAndPrint();
6161

62-
const emittedFiles: string[] = [];
63-
const validFiles = new Set<string>();
62+
const emittedFiles: AbsoluteFsPath[] = [];
63+
const validFiles = new Set<AbsoluteFsPath>();
6464

6565
for (const file of transformedFiles) {
6666
const relativePath = fs.relative(rootDir, fs.resolve(file.fileName));
@@ -86,8 +86,6 @@ function compileTests(fs: FileSystem, test: ComplianceTest): {errors: string[]}
8686
moduleResolution: ts.ModuleResolutionKind.Node16,
8787
module: ts.ModuleKind.Node16,
8888
strict: true,
89-
// TODO: enable once we fix the generated code
90-
noImplicitAny: false,
9189
target: ts.ScriptTarget.ES2015,
9290
types: [],
9391
},

‎packages/compiler-cli/test/compliance/test_cases/isolated/TEST_CASES.json‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,74 @@
1919
]
2020
}
2121
]
22+
},
23+
{
24+
"description": "component with an embedded view",
25+
"inputFiles": ["embedded_view/test.ts"],
26+
"compilationModeFilter": [],
27+
"expectations": [
28+
{
29+
"files": [
30+
{
31+
"expected": "embedded_view/test_transformed.ts",
32+
"generated": "embedded_view/test.ts"
33+
}
34+
]
35+
}
36+
]
37+
},
38+
{
39+
"description": "component with an event listener",
40+
"inputFiles": ["event_listener/test.ts"],
41+
"compilationModeFilter": [],
42+
"expectations": [
43+
{
44+
"files": [
45+
{
46+
"expected": "event_listener/test_transformed.ts",
47+
"generated": "event_listener/test.ts"
48+
},
49+
{
50+
"expected": "event_listener/test.ngtypecheck.ts",
51+
"generated": "event_listener/test.ngtypecheck.ts"
52+
}
53+
]
54+
}
55+
]
56+
},
57+
{
58+
"description": "component with an arrow function",
59+
"inputFiles": ["arrow_function/test.ts"],
60+
"compilationModeFilter": [],
61+
"expectations": [
62+
{
63+
"files": [
64+
{
65+
"expected": "arrow_function/test_transformed.ts",
66+
"generated": "arrow_function/test.ts"
67+
},
68+
{
69+
"expected": "arrow_function/test.ngtypecheck.ts",
70+
"generated": "arrow_function/test.ngtypecheck.ts"
71+
}
72+
]
73+
}
74+
]
75+
},
76+
{
77+
"description": "component using queries",
78+
"inputFiles": ["queries/test.ts"],
79+
"compilationModeFilter": [],
80+
"expectations": [
81+
{
82+
"files": [
83+
{
84+
"expected": "queries/test_transformed.ts",
85+
"generated": "queries/test.ts"
86+
}
87+
]
88+
}
89+
]
2290
}
2391
]
2492
}

‎packages/compiler-cli/test/compliance/test_cases/isolated/arrow_function/test.ngtypecheck.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
…
2+
($event: any /*T:EP*/): any => {
3+
(((this).value /*117,122*/) /*117,122*/).update /*123,129*/(prev /*D:ignore*/ => (prev /*138,142*/) + (1 /*145,146*/) /*138,146*/) /*117,147*/;
4+
};
5+
…

‎packages/compiler-cli/test/compliance/test_cases/isolated/arrow_function/test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import {Component, signal} from '@angular/core';
2+
3+
@Component({
4+
selector: 'test-cmp',
5+
template: '<button (click)="value.update(prev => prev + 1)"></button>',
6+
})
7+
export class TestCmp {
8+
value = signal(1);
9+
}

‎packages/compiler-cli/test/compliance/test_cases/isolated/arrow_function/test_transformed.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
$r3$.ɵɵdefineComponent({
2+
type: TestCmp,
3+
selectors: [["test-cmp"]],
4+
decls: 1,
5+
vars: 0,
6+
consts: [[3, "click"]],
7+
template: function TestCmp_Template(rf: number, ctx: any) {
8+
if (rf & 1) {
9+
$r3$.ɵɵdomElementStart(0, "button", 0);
10+
$r3$.ɵɵdomListener("click", function TestCmp_Template_button_click_0_listener() {
11+
return ctx.value.update((prev: any) => prev + 1);
12+
});
13+
$r3$.ɵɵdomElementEnd();
14+
}
15+
},
16+
encapsulation: 2
17+
});

‎packages/compiler-cli/test/compliance/test_cases/isolated/basic/test_transformed.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
11
…
2-
static ɵfac: i0.ɵɵFactoryDeclaration<TestCmp, never> = function TestCmp_Factory(__ngFactoryType__) { return new (__ngFactoryType__ || TestCmp)(); };
3-
static ɵcmp: i0.ɵɵComponentDeclaration<TestCmp, "test-cmp", never, {}, {}, never, never, true, never> = /*@__PURE__*/ i0.ɵɵdefineComponent({ type: TestCmp, selectors: [["test-cmp"]], decls: 1, vars: 1, template: function TestCmp_Template(rf, ctx) { if (rf & 1) {
4-
i0.ɵɵtext(0);
5-
} if (rf & 2) {
6-
i0.ɵɵtextInterpolate(ctx.x == null ? null : ctx.x.toString());
7-
} }, encapsulation: 2 });
2+
static ɵfac: $r3$.ɵɵFactoryDeclaration<TestCmp, never> = function TestCmp_Factory(__ngFactoryType__: any) {
3+
return new (__ngFactoryType__ || TestCmp)();
4+
};
5+
6+
static ɵcmp: $r3$.ɵɵComponentDeclaration<TestCmp, "test-cmp", never, {}, {}, never, never, true, never> = /*@__PURE__*/
7+
$r3$.ɵɵdefineComponent({
8+
type: TestCmp,
9+
selectors: [["test-cmp"]],
10+
decls: 1,
11+
vars: 1,
12+
template: function TestCmp_Template(rf: number, ctx: any) {
13+
if (rf & 1) {
14+
$r3$.ɵɵtext(0);
15+
}
16+
if (rf & 2) {
17+
$r3$.ɵɵtextInterpolate(ctx.x == null ? null : ctx.x.toString());
18+
}
19+
}, encapsulation: 2 });
820
…

‎packages/compiler-cli/test/compliance/test_cases/isolated/embedded_view/test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import {Component} from '@angular/core';
2+
3+
@Component({
4+
selector: 'test-cmp',
5+
template: '<ng-template>{{x}}</ng-template>',
6+
})
7+
export class TestCmp {
8+
x = 'hello';
9+
}

0 commit comments

Comments
 (0)