immer icon indicating copy to clipboard operation
immer copied to clipboard

feat: add `isNothing()`

Open unional opened this issue 1 year ago • 8 comments

fix #988

I can add tests, but want to know which file should I add it to. Type guard is TS stuff so maybe I create a new file __tests__/common.ts?

unional avatar Oct 30 '22 01:10 unional

Deploy Preview for quizzical-lovelace-dcbd6a canceled.

Name Link
Latest commit 54f63e1cf18d57c4a727675f91a49742a9bf1dd0
Latest deploy log https://app.netlify.com/sites/quizzical-lovelace-dcbd6a/deploys/635dd3788f41e6000957947c

netlify[bot] avatar Oct 30 '22 01:10 netlify[bot]

Another thing is, should the Nothing type be exposed?

Right now I have to access it through typeof nothing

unional avatar Oct 30 '22 01:10 unional

Would you mind to add the test file indeed, testing both the type inference and runtime behavior of the utility?

Sure, do you mind I add type-plus to the dev dependency so that I can easily test the types?

unional avatar Jan 02 '23 20:01 unional

Also, looking at the PR again, it turns out the type is actually not exported. Will need to fix that.

The circular dependency is also an issue. Maybe I can create a different PR to fix all circular dependency issue first, as that would lead to many problems.

unional avatar Jan 03 '23 05:01 unional

Sure, do you mind I add type-plus to the dev dependency so that I can easily test the types?

We already use spec.ts (see e.g. produce.ts), or using // @ts-expected-error might suffice?

mweststrate avatar Jan 03 '23 20:01 mweststrate

I'll check that out. Thx

unional avatar Jan 04 '23 18:01 unional

@unional were you able to set up some tests?

mweststrate avatar Jan 15 '23 16:01 mweststrate

Hi, yes I just get back to this.

I have tried the spec.ts and // @ts-expected-error and saw how you use it:

// @ts-expect-error
assert(value, _ as Nothing)

// vs
isType.equal<false, typeof value, Nothing>()

It works either way. Didn't know about @ts-expect-error. Thanks!.

I do found one issue from this PR. The function isNothing() depends on the Nothing type, but immer implement this mechanism using the a dummy class Nothing.

I looked up the code and found that it is actually needed by one of the public types already:

/** Converts `nothing` into `undefined` */
type FromNothing<T> = T extends Nothing ? undefined : T

/** The inferred return type of `produce` */
export type Produced<Base, Return> = Return extends void
	? Base
	: Return extends Promise<infer Result>
	? Promise<Result extends void ? Base : FromNothing<Result>>
	: FromNothing<Return>

Produced -> FromNothing -> Nothing.

So exposing the type should be needed in the first place and that may be an existing bug. But testing and proving that could be difficult.

However, exposing this Nothing class is not good because it is not used as a class but a type. One way to fix this is replace the mechanism with a branded type, e.g. https://github.com/unional/type-plus/blob/main/ts/nominal-types/Brand.ts#L11

But that will need some investigation.

unional avatar Jan 15 '23 21:01 unional