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

chore(types): separate MatcherContext, MatcherUtils and MatcherState #13141

Merged
merged 6 commits into from
Aug 17, 2022

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Aug 16, 2022

Summary

Inspired by DefinitelyTyped/DefinitelyTyped#61717 which differentiates.

Test plan

Green CI


export type MatcherFunction<Expected extends Array<unknown> = []> =
MatcherFunctionWithState<MatcherState, Expected>;
MatcherFunctionWithContext<MatcherContext, Expected>;

// TODO should be replaced with `MatcherFunctionWithContext`
Copy link
Member Author

Choose a reason for hiding this comment

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

@mrazauskas what is this TODO for?

Copy link
Contributor

@mrazauskas mrazauskas Aug 16, 2022

Choose a reason for hiding this comment

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

If I recall it right, MatcherFunction and MatcherFunctionWithContext does not allow any type of actual and expected values. Or so. Currently there is a need to have anys internally, but would be nicer to rework these into unknowns. After this will be done, RawMatcherFn can be replaced with MatcherFunctionWithContext.

Copy link
Contributor

Choose a reason for hiding this comment

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

In other words, RawMatcherFn is redundant type, but it cannot be removed at this moment.

@@ -41,12 +45,19 @@ export type MatchersObject = {
export type ThrowingMatcherFn = (actual: any) => void;
export type PromiseMatcherFn = (actual: any) => Promise<void>;

export interface MatcherUtils {
Copy link
Member Author

Choose a reason for hiding this comment

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

I just extracted the fields that felt more "util-y" - should I take others?

};

const matcherContext: MatcherContext = {
...getState<MatcherState>(),
Copy link
Member Author

Choose a reason for hiding this comment

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

explicit generic as otherwise it inferred MatcherContext and didn't error when MatcherContext where missing

@SimenB SimenB changed the title chore: separate MatcherContext, MatcherUtils and MatcherState chore(types): separate MatcherContext, MatcherUtils and MatcherState Aug 17, 2022
@SimenB SimenB merged commit a5b52a5 into jestjs:main Aug 17, 2022
@SimenB SimenB deleted the split-up-matcher-context-matcher-utils branch August 17, 2022 06:47
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants