Skip to content

Conversation

jlusty
Copy link
Contributor

@jlusty jlusty commented Apr 20, 2022

Had a chance to have a look into this, and I think I've found the solution to the issue I raised: if there is a prologue (and no super) in a constructor, the prologue won't be skipped when calculating the parameter properties, leading to the prologue being included twice.

Please let me know if I need to update/change anything - I hope I added the tests correctly, all the tests are still passing.

Fixes #48771

parameter property declarations
If there is a prologue (and no super) in a constructor,
the prologue won't be skipped when calculating the
parameter properties, leading to the prologue being
included twice
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Apr 20, 2022
@ghost
Copy link

ghost commented Apr 20, 2022

CLA assistant check
All CLA requirements met.

@jakebailey
Copy link
Member

Would you mind adding in the second test from #48771 (comment)?

Comment on lines -1366 to +1372
statements[0],
...statements.slice(0, superAndPrologueStatementCount),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated so we include parameter properties after any prologues - I believe this is the correct way of doing it, since that's what the existing TS version does

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see also #48687 and #48765, which fixed this for other transforms.

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; notably, #29374 modified three transforms:

  • ts.ts, fixed in #48687
  • es2015.ts, fixed in #48765
  • classFields.ts, fixed in this PR.

I'm hoping that's the last of this bug.

@jakebailey jakebailey merged commit 273a567 into microsoft:main Apr 20, 2022
@jakebailey
Copy link
Member

@typescript-bot cherry-pick this to release-4.6

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 20, 2022

Heya @jakebailey, I've started to run the task to cherry-pick this into release-4.6 on this PR at 891cb6f. You can monitor the build here.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Apr 20, 2022
Component commits:
74c337b Fix handling of prologue statements when there are parameter property declarations If there is a prologue (and no super) in a constructor, the prologue won't be skipped when calculating the parameter properties, leading to the prologue being included twice

bd2a2ec Add second test case

d45098e Update to match old TS version: parameter properties after prologue

891cb6f Parametert properties should be added after any prologue
@typescript-bot
Copy link
Collaborator

Hey @jakebailey, I've opened #48781 for you.

Jack-Works pushed a commit to Jack-Works/TypeScript that referenced this pull request Apr 22, 2022
DanielRosenwasser pushed a commit that referenced this pull request May 5, 2022
Component commits:
74c337b Fix handling of prologue statements when there are parameter property declarations If there is a prologue (and no super) in a constructor, the prologue won't be skipped when calculating the parameter properties, leading to the prologue being included twice

bd2a2ec Add second test case

d45098e Update to match old TS version: parameter properties after prologue

891cb6f Parametert properties should be added after any prologue

Co-authored-by: John Lusty <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing parameter properties when using class with private fields
3 participants