Skip to content

build: enable more strictness flags in non-library code #23077

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 1 commit into from
Jun 29, 2021

Conversation

crisbeto
Copy link
Member

Updates the compiler options for the schematics and other Node-based code, and resolves any compilation errors that showed up as a result. Also fixes a bunch of references to non-existent types from parse5.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 27, 2021
@crisbeto crisbeto marked this pull request as ready for review June 27, 2021 13:53
@crisbeto crisbeto requested review from devversion, jelbourn and a team as code owners June 27, 2021 13:53
@crisbeto crisbeto added merge safe P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Jun 27, 2021
@josephperrott josephperrott removed the request for review from a team June 28, 2021 16:27
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.

Nice! Ideally in the long-term our nested tsconfig's would all inherit the strictness properties from our project bazel-tsconfig-build file, but that is more complicated than it sounds.

@@ -23,23 +23,23 @@ export class AttributeSelectorsMigration extends Migration<UpgradeData> {
data = getVersionUpgradeData(this, 'attributeSelectors');

// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;
enabled: boolean = this.data.length !== 0;
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this is needed. Seems like something TS could infer quite easily?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a weird one, but for some reason TS was flagging the entire class if the type isn't specified explicitly. It was something about not being able to infer the type from the constructor.

Updates the compiler options for the schematics and other Node-based code, and resolves any compilation errors that showed up as a result. Also fixes a bunch of references to non-existent types from `parse5`.
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Jun 29, 2021
@crisbeto crisbeto force-pushed the tools-implicit-overrides branch from d55cb63 to f76ddaa Compare June 29, 2021 14:19
@amysorto amysorto merged commit 7b3006f into angular:master Jun 29, 2021
amysorto pushed a commit that referenced this pull request Jun 29, 2021
Updates the compiler options for the schematics and other Node-based code, and resolves any compilation errors that showed up as a result. Also fixes a bunch of references to non-existent types from `parse5`.

(cherry picked from commit 7b3006f)
@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 30, 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 P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent 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