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): update to tsickle 0.37.1 to fix peerDep warnings #33788

Closed
wants to merge 1 commit into from

Conversation

@IgorMinar
Copy link
Member

IgorMinar commented Nov 13, 2019

tsickle 0.37.1 is compatible with typescript 3.6, so we should use it and fix peerDep warnings from npm/yarn.

I had to update golden files along the way because the output has changed slightly.

And I also had to deal with with a breaking change in tsickle introduced by angular/tsickle@cec4b48

@IgorMinar IgorMinar requested review from angular/dev-infra-framework as code owners Nov 13, 2019
@googlebot googlebot added the cla: yes label Nov 13, 2019
@IgorMinar IgorMinar requested a review from josephperrott Nov 13, 2019
@ngbot ngbot bot modified the milestone: needsTriage Nov 13, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Nov 13, 2019

@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Nov 13, 2019

it seems that a bunch of stuff has changed in tsickle in .1 release :-(

angular/tsickle@5662f72...afb02d5

the decorator downleveling change is likely due to angular/tsickle@cec4b48

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Nov 22, 2019

tsickle 0.37.1 is compatible with typescript 3.6, so we should use it and fix peerDep warnings from npm/yarn.
@IgorMinar IgorMinar force-pushed the IgorMinar:build/tsickle-0.37.1 branch from 9730403 to 76cc7a4 Nov 22, 2019
@IgorMinar IgorMinar requested a review from angular/fw-compiler as a code owner Nov 22, 2019
@@ -45,6 +45,7 @@ ts_library(
"@npm//@bazel/typescript",
"@npm//@types/chokidar",
"@npm//@types/node",
"@npm//minimist",

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Nov 22, 2019

Author Member

@gregmagolan @alexeagle fyi: this is likely one of those "that's how npm works" issue, but after a recent change in tsickle (which being updated in this PR) in which tsickle dropped dependency on minimist, one of our integration tests started failing because minimist in could not be found by compiler-cli.

It turns out that we were missing an explicit dependency on it and were silently depending on tsickle bringing minimist with it. Now that tsickle no longer brings minimist, the oversign in our build graph became visible.

I'm not sure if this is actionable on your end, just wanted to point it out in case this is not an expected behavior.

This comment has been minimized.

Copy link
@gregmagolan

gregmagolan Nov 22, 2019

Contributor

Thanks for the heads up. The failure showed up downstream of this ts_library? Which integration test?
I tried looking at the CircleCI history but can't figure out how to see the old builds for a particular PR in the new UI 🙄

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Nov 22, 2019

Author Member

the target was //packages/compiler-cli/integrationtest:integrationtest

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Nov 22, 2019

@IgorMinar

This comment has been minimized.

Copy link
Member Author

IgorMinar commented Nov 22, 2019

merge-assistance: global approval

matsko added a commit that referenced this pull request Nov 22, 2019
tsickle 0.37.1 is compatible with typescript 3.6, so we should use it and fix peerDep warnings from npm/yarn.

PR Close #33788
@matsko matsko closed this in 3c335c3 Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.