Skip to content

build: update to rules_nodejs v4.0.0-beta.0 and use new API golden tool #23155

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 3 commits into from
Jul 15, 2021

Conversation

devversion
Copy link
Member

@devversion devversion commented Jul 12, 2021

See individual commits (due to a lot of changes files; this is easier for review)

This also contains angular/angular#42814 which should fix our flakiness within RBE

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 12, 2021
@devversion devversion added the in progress This issue is currently in progress label Jul 12, 2021
@devversion devversion force-pushed the nodejs-api-update branch 6 times, most recently from 1e10819 to 069758e Compare July 13, 2021 17:07
@devversion devversion added merge safe target: patch This PR is targeted for the next patch release and removed in progress This issue is currently in progress labels Jul 13, 2021
@devversion devversion marked this pull request as ready for review July 13, 2021 17:20
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

@@ -13,7 +13,7 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {CdkComboboxModule} from './combobox-module';
import {CdkCombobox} from './combobox';
import {dispatchKeyboardEvent, dispatchMouseEvent} from '@angular/cdk/testing/private';
import {dispatchKeyboardEvent, dispatchMouseEvent} from '../../cdk/testing/private';
Copy link
Member Author

@devversion devversion Jul 13, 2021

Choose a reason for hiding this comment

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

Just for reference: There are a couple of similar changes. Such internal files should not be imported through a module name, but rather through a relative source import. This will help with distinguishing between public modules and internal/test-only ones.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

Updates the Bazel NodeJS rules to v4.0.0-beta.0. This is necessary
so that the Angular components repo can start using the new API
golden tool from the shared dev-infra repository, and it's generally
good to stay as up-to-date as possible with the Bazel rules as it's
easy to fall behind, and updating early allows us to discover issues
affecting our tooling earlier (where they are easier to address due to
e.g. potential breaking change policy).

bazel-contrib/rules_nodejs@3be2902.

As part of this change, we remove all manual `module_name`
properties from `ts_library` and `ng_module` targets. These
will be generated now as manually setting the module name is
rather prone to mistakes and this way it also integrates better
with the new `package_name` property introduced as part of
Rules NodeJS v4. This change reveiled that we set a module name
for the private `material/testing` folder. This folder no longer
will resolve through `@angular/material` and needs to be imported
through a relative path.
We recently introduced a new tool for testing the public API
signature of entry-points. This tool is part of the shared
dev-infra package and uses Microsoft's API extractor under the
hood. This commit sets up the new tool and removes ts-api-guardian.
The API goldens have been flattened as part of 766b07e,
but the script for updating goldens has not been updated.

This commit fixes the script so that it respects the new file location
of goldens.
@devversion devversion added action: merge The PR is ready for merge by the caretaker merge: preserve commits When the PR is merged, a rebase and merge should be performed labels Jul 13, 2021
@amysorto amysorto added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jul 15, 2021
@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 Aug 15, 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 merge: preserve commits When the PR is merged, a rebase and merge should be performed target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants