Skip to content

Handle parentless nodes in isParameterPropertyDeclaration #33321

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

Merged

Conversation

mprobst
Copy link
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
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
Member

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

@mprobst
Copy link
Contributor Author

mprobst commented Sep 13, 2019

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

@DanielRosenwasser
Copy link
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
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants