Skip to content

refactor(multiple): enable noImplicitOverride typescript option (#22830) #23091

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
merged 2 commits into from
Jun 30, 2021

Conversation

amysorto
Copy link
Contributor

  • refactor(multiple): enable noImplicitOverride typescript option

Enables the TypeScript noImplicitOverride option and adds
the override keyword where needed.

  • build: add yarn script alias for tsc

Adds a yarn run alias for the tsc binary. Since we have
multiple versions of TypeScript installed, Yarn can pick
another version (e.g. from typescript-4.2) for linking
the tsc binary. This is unexpected because yarn tsc should
resolve to the TypeScript version that is not installed through
a Yarn alias. Changing the order in the Yarn lock file does not
help, and it seems expected in the context of Yarn which does not
know which package (regardless of alias or not) to prefer.

…gular#22830)

* refactor(multiple): enable `noImplicitOverride` typescript option

Enables the TypeScript `noImplicitOverride` option and adds
the `override` keyword where needed.

* build: add yarn script alias for tsc

Adds a `yarn run` alias for the `tsc` binary. Since we have
multiple versions of TypeScript installed, Yarn can pick
another version (e.g. from `typescript-4.2`) for linking
the `tsc` binary. This is unexpected because `yarn tsc` should
resolve to the TypeScript version that is not installed through
a Yarn alias. Changing the order in the Yarn lock file does not
help, and it seems expected in the context of Yarn which does not
know which package (regardless of alias or not) to prefer.
@amysorto amysorto added the target: patch This PR is targeted for the next patch release label Jun 29, 2021
@amysorto amysorto requested review from devversion and mmalerba June 29, 2021 20:24
@amysorto amysorto requested review from andrewseguin, crisbeto, jelbourn and a team as code owners June 29, 2021 20:24
@google-cla

This comment has been minimized.

@google-cla google-cla bot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Jun 29, 2021
@amysorto
Copy link
Contributor Author

Seems like this PR (#22830) was meant for the last minor version. Although it's currently not in the 12.1.x branch. The CircleCI run from merging in #23077 had a linter error which this commit fixes.

…satisfy noImplicitOverride (angular#23017)

Given we merged the `noImplicitOverride` PR together with a couple of
other PRs, new methods could have been introduced that did not use
the `override` keyword. This is the case for the MDC tooltip `_onShow`
method. This commit adds the necessary `override` keyword.
@google-cla

This comment has been minimized.

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. Looks like the issue was that the noImplicitOverride PR still had the target: minor label while 12.1.x already became the latest release train w/ TS 4.3 support.

@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Jun 29, 2021
@devversion
Copy link
Member

@googlebot I consent.

@google-cla google-cla bot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jun 29, 2021
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@amysorto amysorto merged commit 4cdb2f6 into angular:12.1.x Jun 30, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants