Skip to content

Commit

Permalink
Fix class range removal logic after a method overload (#593)
Browse files Browse the repository at this point in the history
Class field handling generates a `rangesToRemove` array describing the tokens
that we should remove as we walk the class, but the logic expected us to always
visit exactly the start token of each range. When a class body has method
overloads, the `processToken` logic to handle that overload (also removing it)
was skipping over one of the ranges to remove, so none of the later ranges to
remove were being matched. To fix, we can use a `>=` comparison to make sure we
handle ranges to remove if they're not already handled.

Possibly a safer approach could be to revisit which ranges to remove we actually
emit so that we can be confident that `processToken` isn't copying them
unexpectedly, but that would require a bit more care, and hopefully this area of
code will go away when class field support is dropped in the future.
  • Loading branch information
alangpierce authored Jan 31, 2021
1 parent 23e5caa commit a1d5776
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/transformers/RootTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,11 @@ export default class RootTransformer {
fieldIndex++;
} else if (
rangeToRemoveIndex < rangesToRemove.length &&
this.tokens.currentIndex() === rangesToRemove[rangeToRemoveIndex].start
this.tokens.currentIndex() >= rangesToRemove[rangeToRemoveIndex].start
) {
this.tokens.removeInitialToken();
if (this.tokens.currentIndex() < rangesToRemove[rangeToRemoveIndex].end) {
this.tokens.removeInitialToken();
}
while (this.tokens.currentIndex() < rangesToRemove[rangeToRemoveIndex].end) {
this.tokens.removeToken();
}
Expand Down
19 changes: 19 additions & 0 deletions test/typescript-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2406,4 +2406,23 @@ describe("typescript transform", () => {
`,
);
});

it("correctly handles field declarations after function overloads", () => {
assertTypeScriptResult(
`
class Class {
method(a: number);
method(a: unknown) {}
declare field: number;
}
`,
`"use strict";
class Class {
method(a) {}
}
`,
);
});
});

0 comments on commit a1d5776

Please # to comment.