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

Add containsMatchingInOrder containsEqualInOrder #2284

Merged
merged 5 commits into from
Oct 2, 2024

Conversation

natebosch
Copy link
Member

@natebosch natebosch commented Sep 27, 2024

The joined behavior in containsInOrder has some usability issues:

  • It mimics the arguments for deepEquals, but it doesn't have the same
    behavior for collection typed elements. Checking that a nested
    collection is contained in order requires a Condition callback that
    uses .deepEquals explicitly.
  • The Object? signature throws away inference on the Condition
    callback arguments. With a method that supports only conditions the
    argument type can be tightened and allow inference.

Deprecate the old containsInOrder and plan to remove it before stable.
This is a bit more restrictive, but it's not too noisy to fit a few
(it) => it.equals(foo) in a collection that needs mixed behavior and
the collection of two methods is less confusing to document than the
joined behavior.

Lean on the "Matches" verb for cases that check a Condition callback
and rename pairwiseComparesTo as pairwiseMatches.

Fix a type check when pretty printing Condition callbacks. Match
more than Condition<dynamic> by checking Condition<Never>.

The joined behavior in `containsInOrder` has some usability issues:
- It mimics the arguments for `deepEquals`, but it doesn't have the same
  behavior for collection typed elements. Checking that a nested
  collection is contained in order requires a `Condition` callback that
  uses `.deepEquals` explicitly.
- The `Object?` signature throws away inference on the `Condition`
  callback arguments. With a method that supports only conditions the
  argument type can be tightened and allow inference.
Copy link

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

Package publish validation ✔️
Package Version Status
package:checks 0.3.1-wip WIP (no publish necessary)
package:test 1.25.9-wip WIP (no publish necessary)
package:test_api 0.7.4-wip WIP (no publish necessary)
package:test_core 0.6.6-wip WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

Also deprecate `pairwiseComparesTo`.
@natebosch natebosch marked this pull request as ready for review September 27, 2024 23:06
@natebosch natebosch requested a review from a team as a code owner September 27, 2024 23:06
@natebosch
Copy link
Member Author

#2257 highlights another place where mixing the matches/equals behavior has a negative impact on the UX. There we are mitigating it by special casing arguments that don't use Condition in some places. I don't think that it makes sense to try to split between deepEquals and deepMatches in that case. The distinction comes down to the deep vs non-deep behavior. With the shallow "contains" check we get a bigger benefit from type inference on the condition callbacks (can allow typed Subject arguments, instead of Subject<dynamic>).

The prior version would accept the `0` as meeting the conditions.
@natebosch natebosch merged commit df3e2f1 into master Oct 2, 2024
56 checks passed
@natebosch natebosch deleted the contains-all-matching branch October 2, 2024 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants