fix(isMatching): support top-level matcher patterns for typed values - #356
Open
xianjianlf2 wants to merge 1 commit into
Open
fix(isMatching): support top-level matcher patterns for typed values#356xianjianlf2 wants to merge 1 commit into
xianjianlf2 wants to merge 1 commit into
Conversation
When calling the two-argument `isMatching(pattern, value)` with a value of a concrete object type, top-level matcher patterns such as `P.instanceOf(...)` failed to type-check. The object branch of `PatternConstraint` intersects with `UnknownProperties` (a string index signature) to allow extra object-pattern keys, but a `Matcher` has no index signature and so wasn't assignable. Add a dedicated overload accepting a top-level `Matcher<unknown, T>`, leaving the object-pattern constraint untouched. Closes gvergnaud#336
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A command handler that threw synchronously escaped yargs' error handling entirely. Unlike a handler that returns a rejected promise (which is routed to
.fail()), a synchronous throw propagated out ofmaybeAsyncResult()uncaught, so.fail()was never invoked.Fix
Wrap the handler invocation so a synchronous throw is routed through
usage.fail()exactly like the asynchronous rejection path, keeping the existing behavior when a parse callback is registered.Testing
test/command.mjscase asserting that a synchronous throw in a command handler reaches.fail().checkvalidation test intest/validation.mjs, which previously relied on the error being swallowed, to assert that the thrown check error now reaches.fail(), consistent with the async path.Closes #1797
Summary by cubic
Fixes type-checking for top-level matcher patterns in
isMatching(pattern, value)when the value has a concrete object type. This enablesP.instanceOf(...),P.string, andP.when(...)to compile and narrow types correctly (fixes #336).isMatchingoverload that accepts a top-levelMatcher<unknown, T>, avoiding theUnknownPropertiesconstraint used for object patterns.P.instanceOf(Foo)andP.instanceOf(Error)and adjust the invalid pattern test.Written for commit b1648e4. Summary will update on new commits.