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

feat(testing/unstable): add type test for mutual assignability #6154

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

KnorpelSenf
Copy link
Contributor

@KnorpelSenf KnorpelSenf commented Oct 28, 2024

If you write complex types, you often have large unions that you want to narrow down according to some rules. Testing these types therefore requires you to check if the union was narrowed down correctly.

When your union types get complex enough, it is no longer feasible to work with Extract because it iterates the union type and checks assignability with each union member. This takes too long for any practical applications. Instead, a better approach is to use type intersections with union types. The TypeScript compiler is very fast at narrowing down such structures.

However, even though the resulting types work correctly, they are structurally different from the types Extract produces. In a nutshell, Extract<string | RegExpMatchArray, string> produces string while (string | RegExpMatchArray) & string isn't simplified further.

Currently, std provides no way of testing these types. Using Has does not check if the union type was narrowed down correctly, and using IsExact fails for intersections of unions despite mutual assignability.

This pull request fills that gap. It adds a new type for type testing called IsMutuallyAssignable. This new type checks if the two provided type parameters are assignable to each other, no matter how they are produced.

This is often less strict than IsExact because the two type parameters are allowed to have a different structure as long as they are assignable to each other. This is often more strict than Has because none of the two type parameters may be a union that contains the other, as this would fail the check for mutual assignability.

// false because E is not assignable to A
assertType<IsMutuallyAssignable<string & RegExpMatchArray, string>>(false);
// false because A is not assignable to E
assertType<IsMutuallyAssignable<string | RegExpMatchArray, string>>(false);
// true because both types are assignable to each other
assertType<IsMutuallyAssignable<string | (string & RegExpMatchArray), string>>(true);

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2024

CLA assistant check
All committers have signed the CLA.

@KnorpelSenf
Copy link
Contributor Author

KnorpelSenf commented Oct 28, 2024

I'm open to renaming the type. It is very descriptive now IMO, but it's also a bit long.

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.52%. Comparing base (ff6037a) to head (ba50803).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6154   +/-   ##
=======================================
  Coverage   96.51%   96.52%           
=======================================
  Files         536      536           
  Lines       40765    40765           
  Branches     6118     6118           
=======================================
+ Hits        39346    39347    +1     
+ Misses       1375     1374    -1     
  Partials       44       44           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kt3k
Copy link
Member

kt3k commented Oct 29, 2024

Sounds a fair suggestion to me.

Can you move this API to the file of the name unstable_types.ts (and link it as unstable-types in testing/deno.json) as we want to separate stable features and new (unstable) features.

@KnorpelSenf
Copy link
Contributor Author

Done!

@kt3k kt3k changed the title feat: add type test for mutual assignability feat(testing/unstable): add type test for mutual assignability Oct 29, 2024
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@kt3k kt3k merged commit b6ab0f4 into denoland:main Oct 29, 2024
20 checks passed
@KnorpelSenf KnorpelSenf deleted the mutual-assignability-check branch October 29, 2024 11:54
@KnorpelSenf
Copy link
Contributor Author

@kt3k can you link me to a place where I can read about the stabilization process? I only found a single sentence in the contribution guide that says that things will be stabilized “after receiving sufficient feedback from the community and the core team” … is there a place where this is being elaborated?

@kt3k
Copy link
Member

kt3k commented Oct 30, 2024

We haven't defined the process yet

@KnorpelSenf
Copy link
Contributor Author

Alright! I'll just follow along with the JSR meetings then

KnorpelSenf added a commit to grammyjs/grammY that referenced this pull request Nov 1, 2024
After denoland/std#6154 was accepted,
we no longer need to rely on our implementation of it. This
PQ replaces it with the implementation from @std.
KnorpelSenf added a commit to grammyjs/grammY that referenced this pull request Nov 1, 2024
After denoland/std#6154 was accepted,
we no longer need to rely on our implementation of it. This
PQ replaces it with the implementation from @std.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants