Skip to content

Commit

Permalink
fix(compiler-cli): support JIT transforms before other transforms mod…
Browse files Browse the repository at this point in the history
…ifying classes (#57262)

Similar to a previous fix that intended to make the JIT transforms
compatible with pre-transforms like e.g. Tsickle, we need to solve
an additional issue where the class properties are synthetic and result
in an `getSourceFile() => undefined` invocation that breaks the import
insertion, causing errors like:

```
TypeError: Cannot read properties of undefined (reading 'fileName')
```

PR Close #57262
  • Loading branch information
devversion authored and thePunderWoman committed Aug 5, 2024
1 parent dbca467 commit e2259c7
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
*/
export const signalInputsTransform: PropertyTransform = (
member,
sourceFile,
host,
factory,
importTracker,
Expand Down Expand Up @@ -61,7 +62,6 @@ export const signalInputsTransform: PropertyTransform = (
'transform': factory.createIdentifier('undefined'),
};

const sourceFile = member.node.getSourceFile();
const newDecorator = factory.createDecorator(
factory.createCallExpression(
createSyntheticAngularCoreDecoratorAccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {createSyntheticAngularCoreDecoratorAccess, PropertyTransform} from './tr
*/
export const signalModelTransform: PropertyTransform = (
member,
sourceFile,
host,
factory,
importTracker,
Expand Down Expand Up @@ -56,7 +57,6 @@ export const signalModelTransform: PropertyTransform = (
),
]);

const sourceFile = member.node.getSourceFile();
const inputDecorator = createDecorator(
'Input',
// Config is cast to `any` because `isSignal` will be private, and in case this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {createSyntheticAngularCoreDecoratorAccess, PropertyTransform} from './tr
*/
export const initializerApiOutputTransform: PropertyTransform = (
member,
sourceFile,
host,
factory,
importTracker,
Expand All @@ -43,7 +44,6 @@ export const initializerApiOutputTransform: PropertyTransform = (
return member.node;
}

const sourceFile = member.node.getSourceFile();
const newDecorator = factory.createDecorator(
factory.createCallExpression(
createSyntheticAngularCoreDecoratorAccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const queryFunctionToDecorator: Record<QueryFunctionName, string> = {
*/
export const queryFunctionsTransforms: PropertyTransform = (
member,
sourceFile,
host,
factory,
importTracker,
Expand All @@ -61,7 +62,6 @@ export const queryFunctionsTransforms: PropertyTransform = (
return member.node;
}

const sourceFile = member.node.getSourceFile();
const callArgs = queryDefinition.call.arguments;
const newDecorator = factory.createDecorator(
factory.createCallExpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function createTransformVisitor(
) {
let hasChanged = false;

const sourceFile = originalNode.getSourceFile();
const members = node.members.map((memberNode) => {
if (!ts.isPropertyDeclaration(memberNode)) {
return memberNode;
Expand All @@ -111,6 +112,7 @@ function createTransformVisitor(
for (const transform of propertyTransforms) {
const newNode = transform(
{...member, node: memberNode},
sourceFile,
host,
ctx.factory,
importTracker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {ImportManager} from '../../../../translator';
/** Function that can be used to transform class properties. */
export type PropertyTransform = (
member: Pick<ClassMember, 'name' | 'accessLevel' | 'value'> & {node: ts.PropertyDeclaration},
sourceFile: ts.SourceFile,
host: ReflectionHost,
factory: ts.NodeFactory,
importTracker: ImportedSymbolsTracker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,52 @@ describe('initializer API metadata transform', () => {
`),
);
});

// Tsickle may transform the class via the downlevel decorator transform in advance in G3.
// In addition, the property declaration may be transformed too, to add `@type` JSDocs in G3.
it('should migrate if the class member and class is transformed in advance', () => {
const fakeDownlevelPreTransform = (ctx: ts.TransformationContext) => {
return (sf: ts.SourceFile) => {
const visitor = (node: ts.Node) => {
// Updating the `{transform: () => {}} arrow function triggers both transforms.
if (ts.isArrowFunction(node)) {
return ctx.factory.updateArrowFunction(
node,
node.modifiers,
node.typeParameters,
node.parameters,
node.type,
node.equalsGreaterThanToken,
ctx.factory.createBlock([]),
);
}
return ts.visitEachChild(node, visitor, ctx);
};
return ts.visitEachChild(sf, visitor, ctx);
};
};

const result = transform(
`
import {input, Directive} from '@angular/core';
@Directive({})
class MyDir {
someInput = input({transform: () => {}});
}
`,
false,
fakeDownlevelPreTransform,
);

expect(result).toContain(
omitLeadingWhitespace(`
__decorate([
i0.Input({ isSignal: true, alias: "someInput", required: false, transform: undefined })
], MyDir.prototype, "someInput", void 0);
`),
);
});
});

describe('model()', () => {
Expand Down

0 comments on commit e2259c7

Please sign in to comment.