Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler): account for an existing constructor in convert-decorators #3776

Merged
merged 3 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -372,18 +372,32 @@ export const updateConstructor = (
const constructorMethod = classMembers[constructorIndex];

if (constructorIndex >= 0 && ts.isConstructorDeclaration(constructorMethod)) {
const hasSuper = constructorMethod.body.statements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);
const constructorBodyStatements: ts.NodeArray<ts.Statement> =
constructorMethod.body?.statements ?? ts.factory.createNodeArray();
const hasSuper = constructorBodyStatements.some((s) => s.kind === ts.SyntaxKind.SuperKeyword);

if (!hasSuper && needsSuper(classNode)) {
statements = [createConstructorBodyWithSuper(), ...statements];
// if there is no super and it needs one the statements comprising the
// body of the constructor should be:
//
// 1. the `super()` call
// 2. the new statements we've created to initialize fields
// 3. the statements currently comprising the body of the constructor
statements = [createConstructorBodyWithSuper(), ...statements, ...constructorBodyStatements];
} else {
// if no super is needed then the body of the constructor should be:
//
// 1. the new statements we've created to initialize fields
// 2. the statements currently comprising the body of the constructor
statements = [...statements, ...constructorBodyStatements];
}

classMembers[constructorIndex] = ts.factory.updateConstructorDeclaration(
constructorMethod,
constructorMethod.decorators,
constructorMethod.modifiers,
constructorMethod.parameters,
ts.factory.updateBlock(constructorMethod.body, statements)
ts.factory.updateBlock(constructorMethod?.body ?? ts.factory.createBlock([]), statements)
);
} else {
// we don't seem to have a constructor, so let's create one and stick it
Expand All @@ -393,12 +407,7 @@ export const updateConstructor = (
}

classMembers = [
ts.factory.createConstructorDeclaration(
undefined,
undefined,
undefined,
ts.factory.createBlock(statements, true)
),
ts.factory.createConstructorDeclaration(undefined, undefined, [], ts.factory.createBlock(statements, true)),
...classMembers,
];
}
Expand Down
113 changes: 112 additions & 1 deletion src/compiler/transformers/test/convert-decorators.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,118 @@ describe('convert-decorators', () => {
);
});

it('should not add a super call to the constructor if necessary', () => {
it('should preserve statements in an existing constructor', () => {
const t = transpileModule(`
@Component({
tag: 'my-component',
})
export class MyComponent {
constructor() {
console.log('boop');
}
}`);

expect(t.outputText).toBe(
c`export class MyComponent {
constructor() {
console.log('boop');
}

static get is() {
return "my-component";
}}`
);
});

it('should preserve statements in an existing constructor w/ @Prop', () => {
const t = transpileModule(`
@Component({
tag: 'my-component',
})
export class MyComponent {
@Prop() count: number;

constructor() {
console.log('boop');
}
}`);

expect(t.outputText).toContain(
c`constructor() {
this.count = undefined;
console.log('boop');
}`
);
});

it('should allow user to initialize field in an existing constructor w/ @Prop', () => {
const t = transpileModule(`
@Component({
tag: 'my-component',
})
export class MyComponent {
@Prop() count: number;

constructor() {
this.count = 3;
}
}`);

// the initialization we do to `undefined` (since no value is present)
// should be before the user's `this.count = 3` to ensure that their code
// wins.
expect(t.outputText).toContain(
c`constructor() {
this.count = undefined;
this.count = 3;
}`
);
});

it('should preserve statements in an existing constructor w/ non-decorated field', () => {
const t = transpileModule(`
@Component({
tag: 'example',
})
export class Example implements FooBar {
private classProps: Array<string>;

constructor() {
this.classProps = ["variant", "theme"];
}
}`);

expect(t.outputText).toBe(
c`export class Example {
constructor() {
this.classProps = ["variant", "theme"];
}}`
);
});

it('should preserve statements in an existing constructor super, decorated field', () => {
const t = transpileModule(`
@Component({
tag: 'example',
})
export class Example extends Parent {
@Prop() foo: string = "bar";

constructor() {
console.log("hello!")
}
}`);

expect(t.outputText).toContain(
c`constructor() {
super();
this.foo = "bar";
console.log("hello!");
}`
);
});

it('should not add a super call to the constructor if not necessary', () => {
const t = transpileModule(`
@Component({tag: 'cmp-a'})
export class CmpA implements Foobar {
Expand Down