Skip to content

Handle parentless nodes in isParameterPropertyDeclaration#33321

Merged
DanielRosenwasser merged 1 commit intomicrosoft:release-3.6from
mprobst:transformParameterProperty
Sep 17, 2019
Merged

Handle parentless nodes in isParameterPropertyDeclaration#33321
DanielRosenwasser merged 1 commit intomicrosoft:release-3.6from
mprobst:transformParameterProperty

Conversation

@mprobst
Copy link
Copy Markdown
Contributor

@mprobst mprobst commented Sep 9, 2019

Fixes #33295.

This follows a similar pattern as in #20314 by requiring an explicit
parent parameter. Where possible, it uses the appopriate variable at
the call sites.

In several locations there is no context available though (e.g.
inspecting valueDeclarations) and we access .parent as the code
previously did. From a cursory inspection this seems correct, these
callpaths originate in phases where there must be a parent (i.e. in
checker, binder, etc).

Change-Id: I28e4726777b57237bec776e4001e9e69ac591b11

@mprobst
Copy link
Copy Markdown
Contributor Author

mprobst commented Sep 9, 2019

CC @rbuckton and @weswigham, whose PR I'm shamelessly copying here :-)

Also fixes angular/tsickle#1065.

@mprobst mprobst force-pushed the transformParameterProperty branch from d3d2895 to f272ec2 Compare September 9, 2019 12:14
@weswigham weswigham requested a review from rbuckton September 9, 2019 18:18
@weswigham
Copy link
Copy Markdown
Member

@DanielRosenwasser you proooobably want to accept this and merge into a 3.6 patch release, assuming there will be one.

@mprobst
Copy link
Copy Markdown
Contributor Author

mprobst commented Sep 13, 2019

@weswigham / @DanielRosenwasser what's the next step here, do I need to do something?

@DanielRosenwasser
Copy link
Copy Markdown
Member

DanielRosenwasser commented Sep 13, 2019

Sorry, thought this was merged because I was on mobile. Can you fix conflicts and create a separate PR to release-3.6?

Fixes microsoft#33295.

This follows a similar pattern as in microsoft#20314 by requiring an explicit
`parent` parameter. Where possible, it uses the appopriate variable at
the call sites.

In several locations there is no context available though (e.g.
inspecting `valueDeclarations`) and we access `.parent` as the code
previously did. From a cursory inspection this seems correct, these
callpaths originate in phases where there must be a `parent` (i.e. in
checker, binder, etc).

Change-Id: I28e4726777b57237bec776e4001e9e69ac591b11
@mprobst mprobst force-pushed the transformParameterProperty branch from cee129d to e8be5e8 Compare September 17, 2019 15:11
@typescript-bot
Copy link
Copy Markdown
Collaborator

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

@mprobst mprobst changed the base branch from master to release-3.6 September 17, 2019 15:12
@mprobst
Copy link
Copy Markdown
Contributor Author

mprobst commented Sep 17, 2019

@DanielRosenwasser done, this is now a single commit on top of release-3.6.

@DanielRosenwasser DanielRosenwasser merged commit 46ccaa2 into microsoft:release-3.6 Sep 17, 2019
@mprobst mprobst deleted the transformParameterProperty branch March 9, 2020 08:34
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants