GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fixed multiple prologue directives with parameter properties by JoshuaKGoldberg · Pull Request #48687 · microsoft/TypeScript

Conversation

@JoshuaKGoldberg

@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Apr 13, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Two issues:

  • Parameter properties should be inserted after any prologue directives
  • Prologue directives were getting copied over twice when there wasn't a super()

Fixes #48671

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@JoshuaKGoldberg
Fixed multiple prologue directives with parameter properties
76f482f
@typescript-bot TypeScript Bot (typescript-bot) assigned Jake Bailey (jakebailey) Apr 13, 2022
@typescript-bot TypeScript Bot (typescript-bot) added the For Milestone Bug PRs that fix a bug with a specific milestone label Apr 13, 2022
JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Apr 13, 2022
View reviewed changes
Comment thread src/compiler/checker.ts
break;
}
if (!isPrologueDirective(statement) && nodeImmediatelyReferencesSuperOrThis(statement)) {
if (nodeImmediatelyReferencesSuperOrThis(statement)) {

@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) Apr 13, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This change isn't necessary for the PR; I just had only noticed the check is unnecessary after #29374 was merged 😄

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) commented Apr 13, 2022
View reviewed changes
Comment thread src/compiler/transformers/ts.ts
const indexAfterLastPrologueStatement = factory.copyPrologue(body.statements, statements, /*ensureUseStrict*/ false, visitor);
const superStatementIndex = findSuperStatementIndex(body.statements, indexAfterLastPrologueStatement);
const prologueStatementCount = factory.copyPrologue(body.statements, statements, /*ensureUseStrict*/ false, visitor);
const superStatementIndex = findSuperStatementIndex(body.statements, prologueStatementCount);

@JoshuaKGoldberg Josh Ghoulberg 👻 (JoshuaKGoldberg) Apr 13, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm nitpicking the variable name here. Both variable names are correct; prologueStatementCount just makes more sense to me -- especially in the context of the new array spread below.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) marked this pull request as ready for review April 13, 2022 23:44
jakebailey
Jake Bailey (jakebailey) approved these changes Apr 14, 2022
View reviewed changes
@jakebailey
Jake Bailey (jakebailey) merged commit fa2515e into microsoft:main Apr 14, 2022
@jakebailey

Jake Bailey (jakebailey) commented Apr 14, 2022

Copy link
Copy Markdown
Member

Thanks for fixing this!
JoshuaKGoldberg reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JoshuaKGoldberg
Josh Ghoulberg 👻 (JoshuaKGoldberg) deleted the fix-parameter-properties-and-prologue-directives branch April 14, 2022 16:34
@p-szm P Sz (p-szm) mentioned this pull request Apr 19, 2022
Wrong constructor code generated when target is ES5 #48761
Closed
This was referenced Apr 19, 2022
Missing parameter properties when using class with private fields #48771
Closed
Fix handling of prologue statements when there are parameter property declarations #48775
Merged
@jakebailey

Jake Bailey (jakebailey) commented Apr 20, 2022

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) cherry-pick this to release-4.6
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the task to cherry-pick this into release-4.6 on this PR at 76f482f. You can monitor the build here.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-4.6 manually.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Jake Bailey (jakebailey) pushed a commit to jakebailey/TypeScript that referenced this pull request Apr 20, 2022
@JoshuaKGoldberg @jakebailey
Fixed multiple prologue directives with parameter properties (microso…
fc3d63e 
…ft#48687)
@jakebailey

Jake Bailey (jakebailey) commented Apr 20, 2022

Copy link
Copy Markdown
Member

Sent #48782 manually. This PR is needed for the other PRs thanks to this PR affecting the base ts.ts transform.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Jack Works (Jack-Works) pushed a commit to Jack-Works/TypeScript that referenced this pull request Apr 22, 2022
@JoshuaKGoldberg @Jack-Works
Fixed multiple prologue directives with parameter properties (microso…
ae6696f 
…ft#48687)
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request May 3, 2022
Pick PR #48687 to release-4.6 #48782
Merged
Daniel Rosenwasser (DanielRosenwasser) pushed a commit that referenced this pull request May 3, 2022
@jakebailey @JoshuaKGoldberg
Fixed multiple prologue directives with parameter properties (#48687) (…
81f1e5f 
…#48782)

Co-authored-by: Josh Goldberg <[email protected]>
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jakebailey Jake Bailey (jakebailey) jakebailey approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@jakebailey Jake Bailey (jakebailey)

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No-op string in constructor is repeated and reordered when constructor assignment is used

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@JoshuaKGoldberg @jakebailey @typescript-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.