rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

feat(@bazel/typescript): allow typescript 4.4 in dep

Open longlho opened this issue 2 years ago • 14 comments

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] Tests for the changes have been added (for bug fixes / features)
  • [x] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [ ] Bugfix
  • [ ] Feature (please, look at the "Scope of the project" section in the README.md file)
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

longlho avatar Oct 18 '21 03:10 longlho

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla[bot] avatar Oct 20 '21 02:10 google-cla[bot]

@googlebot I consent.

alexeagle avatar Oct 20 '21 02:10 alexeagle

I added a commit to test the upgrade, and TS 4.4 seems to break the custom TS compiler in the ts_library rule :(

alexeagle avatar Oct 20 '21 02:10 alexeagle

hmm let me see if I can fix that

longlho avatar Oct 20 '21 03:10 longlho

Sadly I'm sure someone at google already fixed it on their fork- thanks!

alexeagle avatar Oct 20 '21 04:10 alexeagle

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla[bot] avatar Oct 22 '21 14:10 google-cla[bot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla[bot] avatar Oct 22 '21 14:10 google-cla[bot]

looks like I might have to bump angular as well which is kinda a rabbit hole :-/

longlho avatar Oct 22 '21 14:10 longlho

yeah that's sad and not that surprising. they both rely on a lot of TS internals. thanks for trying, we'll get to this eventually...

alexeagle avatar Oct 22 '21 15:10 alexeagle

@devversion any chance you have time to get this angular example upgrade working so we can use TS 4.4?

alexeagle avatar Dec 16 '21 00:12 alexeagle

@alexeagle hey, yeah. I can try having a look over the holidays!

It's a little unclear to me what the canonical Bazel example should look like. As of v13, either apps would need to create UMD bundles for Angular NPM packages on their own, or use a bundled like esbuild for tests, dev-app etc. In Angular/components we combine both but that is pretty advanced. I'm leaning towards the esbuild approach as code is already bundled in the example as it seems.

Just throwing this out in case you have any strong feelings

devversion avatar Dec 21 '21 22:12 devversion

https://github.com/bazelbuild/rules_nodejs/pull/3191

lencioni avatar Jan 06 '22 16:01 lencioni

I think it's essentially fixed by #3191 since @bazel/typescript has the wider peer dep. The rest of this change is still stuck :(

alexeagle avatar Jan 06 '22 18:01 alexeagle

This Pull Request has been automatically marked as stale because it has not had any activity for 6 months. It will be closed if no further activity occurs in 30 days. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Jul 23 '22 03:07 github-actions[bot]

This PR was automatically closed because it went 30 days without any activity since it was labeled "Can Close?"

github-actions[bot] avatar Aug 22 '22 03:08 github-actions[bot]