Skip to content
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

fix(bazel): reenable template type checking in ng_module #34144

Closed

Conversation

@IgorMinar
Copy link
Member

IgorMinar commented Nov 29, 2019

due to an unfortunate condition in

// Skip template type-checking if it's disabled.
if (this.options.ivyTemplateTypeCheck === false &&
this.options.fullTemplateTypeCheck !== true) {
return [];
}
the typechecking has been disabled when running under bazel + ivy.

As far as I can tell the ivyTemplateTypeCheck flag is now obsolete, so removing this
code from ng_module.bzl is desirable. I'll send a separate PR to remove the flag completely.

due to an unfortunate condition in https://github.com/angular/angular/blob/168abc6d6f52713383411b14980e104c99bfeef5/packages/compiler-cli/src/ngtsc/program.ts#L430-L434 the typechecking has been disabled when running under bazel + ivy.

As far as I can tell the ivyTemplateTypeCheck flag is now obsolete, so removing this
code from ng_module.bzl is desirable. I'll send a separate PR to remove the flag completely.
@googlebot googlebot added the cla: yes label Nov 29, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Nov 29, 2019

@IgorMinar IgorMinar marked this pull request as ready for review Nov 29, 2019
@IgorMinar IgorMinar requested review from angular/tools-bazel as code owners Nov 29, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 29, 2019
@IgorMinar IgorMinar requested a review from josephperrott Nov 29, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Nov 29, 2019

@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Nov 29, 2019

@IgorMinar IgorMinar requested a review from alxhub Nov 29, 2019
@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Nov 29, 2019

This fails with lots of typechecking errors in g3. @alxhub is this expected? is typechecking disabled in our g3 presubmits?

@IgorMinar IgorMinar force-pushed the IgorMinar:bazel/fix-ng_module-typecheck branch from 2f6a794 to 5b8dc32 Dec 1, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 1, 2019

@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Dec 1, 2019

merge-assistance: requires ivy flip CL sync once this PR is submitted to g3

IgorMinar added 2 commits Nov 29, 2019
…e repo

Various targets have their template type-checking disabled in the past.

There is no reason for this any more.

The only target that was tricky was packages/examples/core:core_examples
which was quite broken and I had to fix it up.

Template typechecking is still disabled under blaze, see FW-1753 for more
info.
…-service/test

This target will need to be completely updated once the language service supports Ivy.
@IgorMinar IgorMinar force-pushed the IgorMinar:bazel/fix-ng_module-typecheck branch from 5b8dc32 to 0aafbf6 Dec 1, 2019
@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Dec 1, 2019

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 1, 2019

@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Dec 1, 2019

I've reverted this change for blaze and reenabled the template type check just for bazel for now. I also created a Jira for the g3 issue as a follow up task: FW-1753

Copy link
Member

josephperrott left a comment

LGTM, global approval

@IgorMinar IgorMinar removed the request for review from alxhub Dec 2, 2019
mhevery added a commit that referenced this pull request Dec 2, 2019
due to an unfortunate condition in https://github.com/angular/angular/blob/168abc6d6f52713383411b14980e104c99bfeef5/packages/compiler-cli/src/ngtsc/program.ts#L430-L434 the typechecking has been disabled when running under bazel + ivy.

As far as I can tell the ivyTemplateTypeCheck flag is now obsolete, so removing this
code from ng_module.bzl is desirable. I'll send a separate PR to remove the flag completely.

PR Close #34144
mhevery added a commit that referenced this pull request Dec 2, 2019
…e repo (#34144)

Various targets have their template type-checking disabled in the past.

There is no reason for this any more.

The only target that was tricky was packages/examples/core:core_examples
which was quite broken and I had to fix it up.

Template typechecking is still disabled under blaze, see FW-1753 for more
info.

PR Close #34144
mhevery added a commit that referenced this pull request Dec 2, 2019
…-service/test (#34144)

This target will need to be completely updated once the language service supports Ivy.

PR Close #34144
@mhevery mhevery closed this in d8792b3 Dec 2, 2019
mhevery added a commit that referenced this pull request Dec 2, 2019
…e repo (#34144)

Various targets have their template type-checking disabled in the past.

There is no reason for this any more.

The only target that was tricky was packages/examples/core:core_examples
which was quite broken and I had to fix it up.

Template typechecking is still disabled under blaze, see FW-1753 for more
info.

PR Close #34144
mhevery added a commit that referenced this pull request Dec 2, 2019
…-service/test (#34144)

This target will need to be completely updated once the language service supports Ivy.

PR Close #34144
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

angular-automatic-lock-bot bot commented Jan 2, 2020

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 Jan 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.