fix(cdk/testing): require at least one argument for locator functions #23619

crisbeto · 2021-09-22T20:10:21Z

Currently locator functions accept a spread argument which technically allows for zero selectors to be passed in. This can result in weird runtime errors.

These changes add a runtime error if no selectors are passed in.

Note that a previous iteration of these changes tried to enforce this with typings, but it resulted in breaking changes.

crisbeto · 2021-09-22T20:26:35Z

src/cdk/testing/component-harness.ts

@@ -25,6 +25,9 @@ export type AsyncOptionPredicate<T, O> = (item: T, option: O) => Promise<boolean
 export type HarnessQuery<T extends ComponentHarness> =
    ComponentHarnessConstructor<T> | HarnessPredicate<T>;

+/** Queries that can be passed into a locator function. */
+export type LocatorQueries = [HarnessQuery<any> | string, ...(HarnessQuery<any> | string)[]];


I'm not a fan of the name, but I couldn't come up with something better. I also don't want to do it inline since it's a little long.

mmalerba

LGTM

crisbeto · 2021-10-29T12:35:42Z

I looked into the g3 failures for this PR and they are all caused by the fact that the new method signature doesn't allow for the spread operator to be used. E.g. there is code like:

locatorFactory.locatorForAll(...arrayOfPredicates);

Which causes this error:

A spread argument must either have a tuple type or be passed to a rest parameter. ts(2556)

I tried a few alternate approaches like moving the spread argument to the front of LocatorQueries, but it didn't help.

jelbourn · 2021-11-01T22:34:02Z

Should we just change this to a runtime error, then?

Currently locator functions accept a spread argument which technically allows for zero selectors to be passed in. This can result in weird runtime errors. These changes add a runtime error if no selectors are passed in. Note that a previous iteration of these changes tried to enforce this with typings, but it resulted in breaking changes.

crisbeto · 2022-03-07T08:26:30Z

Changed to a runtime error in order to unblock it.

…#23619) Currently locator functions accept a spread argument which technically allows for zero selectors to be passed in. This can result in weird runtime errors. These changes add a runtime error if no selectors are passed in. Note that a previous iteration of these changes tried to enforce this with typings, but it resulted in breaking changes. (cherry picked from commit f592a43)

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`13.2.5` -> `13.2.6`](https://renovatebot.com/diffs/npm/@angular%2fcdk/13.2.5/13.2.6) | | [@angular/material](https://github.com/angular/components) | dependencies | patch | [`13.2.5` -> `13.2.6`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/13.2.5/13.2.6) | --- ### Release Notes <details> <summary>angular/components</summary> ### [`v13.2.6`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#1326-suede-spaghetti-2022-03-09) [Compare Source](angular/components@13.2.5...13.2.6) ##### cdk | Commit | Type | Description | | -- | -- | -- | | [39929a815d](angular/components@39929a8) | fix | **overlay:** backdrop timeouts not being cleared in some cases ([#23972](angular/components#23972)) | | [2f2b0c7cf4](angular/components@2f2b0c7) | fix | **testing:** dispatch mouseover and mouseout events in UnitTestElement ([#24490](angular/components#24490)) | | [edca54f2d0](angular/components@edca54f) | fix | **testing:** require at least one argument for locator functions ([#23619](angular/components#23619)) | ##### material | Commit | Type | Description | | -- | -- | -- | | [c4993ac171](angular/components@c4993ac) | fix | **button:** avoid setting a tabindex on all link buttons ([#22901](angular/components#22901)) | | [c47d30e0e5](angular/components@c47d30e) | fix | **dialog:** don't wait for animation before moving focus ([#24121](angular/components#24121)) | | [70b8248568](angular/components@70b8248) | fix | **expansion:** able to tab into descendants with visibility while closed ([#24045](angular/components#24045)) | | [d22d73ab8d](angular/components@d22d73a) | fix | **select:** disabled state out of sync when swapping form group with a disabled one ([#17872](angular/components#17872)) | | [911d6b71d4](angular/components@911d6b7) | fix | **slide-toggle:** clear name from host node ([#15505](angular/components#15505)) | | [4b5363d160](angular/components@4b5363d) | fix | **tooltip:** decouple removal logic from change detection ([#19432](angular/components#19432)) | ##### material-experimental | Commit | Type | Description | | -- | -- | -- | | [8414646d79](angular/components@8414646) | fix | **mdc-card:** remove extra margin if header doesn't have an avatar ([#19072](angular/components#19072)) | | [f66486dc5b](angular/components@f66486d) | fix | **mdc-slider:** fix a few null pointer exceptions ([#23659](angular/components#23659)) | ##### multiple | Commit | Type | Description | | -- | -- | -- | | [6ee0089ce6](angular/components@6ee0089) | fix | don't block child component animations on open ([#24529](angular/components#24529)) | #### Special Thanks Andrew Seguin, Jeri Peier, Kristiyan Kostadinov and Paul Gschwendtner  </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1214 Reviewed-by: Epsilon_02 <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>

…angular#23619) Currently locator functions accept a spread argument which technically allows for zero selectors to be passed in. This can result in weird runtime errors. These changes add a runtime error if no selectors are passed in. Note that a previous iteration of these changes tried to enforce this with typings, but it resulted in breaking changes.

angular-automatic-lock-bot · 2022-04-07T00:03:41Z

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.}

google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 22, 2021

crisbeto mentioned this pull request Sep 22, 2021

fix(cdk/testing): require at least one argument locator functions #23384

Closed

crisbeto marked this pull request as ready for review September 22, 2021 20:25

crisbeto requested review from devversion, jelbourn, mmalerba and a team as code owners September 22, 2021 20:25

crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Sep 22, 2021

crisbeto commented Sep 22, 2021

View reviewed changes

mmalerba approved these changes Sep 22, 2021

View reviewed changes

mmalerba added the action: merge The PR is ready for merge by the caretaker label Sep 22, 2021

devversion approved these changes Oct 8, 2021

View reviewed changes

crisbeto force-pushed the testing-min-params branch from 8a654f9 to b4cb20f Compare October 28, 2021 12:26

crisbeto added blocked This issue is blocked by some external factor, such as a prerequisite PR and removed action: merge The PR is ready for merge by the caretaker labels Oct 29, 2021

andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 28, 2021

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2021

crisbeto force-pushed the testing-min-params branch from b4cb20f to 1a34a05 Compare March 7, 2022 08:26

crisbeto added action: merge The PR is ready for merge by the caretaker and removed blocked This issue is blocked by some external factor, such as a prerequisite PR labels Mar 7, 2022

crisbeto merged commit f592a43 into angular:master Mar 7, 2022

sync-by-unito bot mentioned this pull request Mar 14, 2022

build(deps): bump @angular/cdk from 12.2.4 to 13.2.6 numbersprotocol/capture-lite#1370

Closed

sync-by-unito bot mentioned this pull request Mar 22, 2022

build(deps): bump @angular/cdk from 12.2.4 to 13.3.0 numbersprotocol/capture-lite#1403

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cdk/testing): require at least one argument for locator functions #23619

fix(cdk/testing): require at least one argument for locator functions #23619

crisbeto commented Sep 22, 2021 •

edited

Loading

crisbeto Sep 22, 2021

mmalerba left a comment

crisbeto commented Oct 29, 2021 •

edited

Loading

jelbourn commented Nov 1, 2021

crisbeto commented Mar 7, 2022

angular-automatic-lock-bot bot commented Apr 7, 2022

fix(cdk/testing): require at least one argument for locator functions #23619

fix(cdk/testing): require at least one argument for locator functions #23619

Conversation

crisbeto commented Sep 22, 2021 • edited Loading

crisbeto Sep 22, 2021

Choose a reason for hiding this comment

mmalerba left a comment

Choose a reason for hiding this comment

crisbeto commented Oct 29, 2021 • edited Loading

jelbourn commented Nov 1, 2021

crisbeto commented Mar 7, 2022

angular-automatic-lock-bot bot commented Apr 7, 2022

crisbeto commented Sep 22, 2021 •

edited

Loading

crisbeto commented Oct 29, 2021 •

edited

Loading