Skip to content

build: update to nodejs rules 0.41.0#33996

Closed
gregmagolan wants to merge 2 commits intoangular:masterfrom
gregmagolan:rules_nodejs_0.41.0
Closed

build: update to nodejs rules 0.41.0#33996
gregmagolan wants to merge 2 commits intoangular:masterfrom
gregmagolan:rules_nodejs_0.41.0

Conversation

@gregmagolan
Copy link
Contributor

@gregmagolan gregmagolan commented Nov 22, 2019

This brings is changes to the @nodejs repository required for #33927. See release notes for more details: https://github.com/bazelbuild/rules_nodejs/releases/tag/0.41.0.

rules_nodejs is approaching 1.0 and breaking changes for that release are being made more frequently. In this release, the ts_devserver API changed and it no longer injects html script tags into a provided index.html file. The diff on this commit is large as this breaking change affects quite a few tests.

Also note that we don’t update @angular/bazel schematics and integration/bazel as 0.41.0 is not a recommended update for angular end-users yet due to the breaking changes in ts_devserver & web_package (now named pkg_web). When a suitable plain npm package that is in progress is finished then it will be possible to easily replace the html injection functionality removed from ts_devserver & pkg_web.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure 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
  • No

Other information

@gregmagolan
Copy link
Contributor Author

gregmagolan commented Nov 24, 2019

Note: protractor data work-around in this PR can be removed after this bazel-contrib/rules_nodejs#1388 lands and makes it into the next release.

@gregmagolan gregmagolan added target: patch This PR is targeted for the next patch release area: bazel Issues related to the published `@angular/bazel` build rules labels Nov 24, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 24, 2019
@gregmagolan gregmagolan added the area: build & ci Related the build and CI infrastructure of the project label Nov 24, 2019
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM, global approval

This bring is changes to the @nodejs repository required for #33927. See release notes for more details: https://github.com/bazelbuild/rules_nodejs/releases/tag/0.41.0.

rules_nodejs is approaching 1.0 and breaking changes for that release are being made more frequently. In this release, the ts_devserver API changed and it no longer injects html script tags into a provided index.html file. The diff on this commit is large as this breaking change affects quite a few tests.

Also note that we don’t update @angular/bazel schematics and integration/bazel as 0.41.0 is not a recommended update for angular users yet due to the breaking changes in ts_devserver & web_package (now named pkg_web). When a suitable plain npm package that is in progress is finished then it will be possible to easily replace the html injection functionality removed from ts_devserver & pkg_web.
@alexeagle alexeagle added the action: merge The PR is ready for merge by the caretaker label Nov 26, 2019
@josephperrott josephperrott added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Nov 26, 2019
@josephperrott
Copy link
Member

Caretaker note: Global Approval

mhevery pushed a commit that referenced this pull request Nov 27, 2019
This bring is changes to the @nodejs repository required for #33927. See release notes for more details: https://github.com/bazelbuild/rules_nodejs/releases/tag/0.41.0.

rules_nodejs is approaching 1.0 and breaking changes for that release are being made more frequently. In this release, the ts_devserver API changed and it no longer injects html script tags into a provided index.html file. The diff on this commit is large as this breaking change affects quite a few tests.

Also note that we don’t update @angular/bazel schematics and integration/bazel as 0.41.0 is not a recommended update for angular users yet due to the breaking changes in ts_devserver & web_package (now named pkg_web). When a suitable plain npm package that is in progress is finished then it will be possible to easily replace the html injection functionality removed from ts_devserver & pkg_web.

PR Close #33996
mhevery pushed a commit that referenced this pull request Nov 27, 2019
@mhevery mhevery closed this in c4335e2 Nov 27, 2019
mhevery pushed a commit that referenced this pull request Nov 27, 2019
@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 Dec 28, 2019
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 area: bazel Issues related to the published `@angular/bazel` build rules area: build & ci Related the build and CI infrastructure of the project cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants