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

null/undefined initial atom value confuses typescript #550

Closed
andrewagain opened this issue Jun 23, 2021 · 14 comments
Closed

null/undefined initial atom value confuses typescript #550

andrewagain opened this issue Jun 23, 2021 · 14 comments

Comments

@andrewagain
Copy link

andrewagain commented Jun 23, 2021

Whenever I set null or undefined as the initial value on an atom, TypeScript infers the type as Atom instead of WritableAtom.

Working case: initial value is false so type is shown as WritableAtom:
Screen Shot 2021-06-23 at 3 12 58 PM

Non-working case: initial value is null so type is just Atom:

Screen Shot 2021-06-23 at 3 13 22 PM

This shows up as a type error on useUpdateAtom:
Screen Shot 2021-06-23 at 3 13 15 PM

My current workaround: caste to PrimitiveAtom:

const myAtomConfig = atom<boolean | null>(null) as PrimitiveAtom<
  boolean | null
>;
@dai-shi
Copy link
Member

dai-shi commented Jun 23, 2021

Oh... Nice catch. Do you happen to know how to fix??

@andrewagain
Copy link
Author

Looks like TypeScript is choosing the wrong overload for atom(). It is choosing between these two:

export declare function atom<Value>(read: Read<Value>): Atom<Value>;
export declare function atom<Value>(initialValue: Value): [Value] extends [Function] ? never : PrimitiveAtom<Value> & WithInitialValue<Value>;

When we have a non-null like atom("hi") it can see that "hi" is not a function and chooses the correct overload (the second one).

But when we have atom(null) it matches the first overload (the one for read-only derived atoms) and uses that because it appears first.

I couldn't figure out how to tell TypeScript that null does not qualify as Read<Value>, but I can fix it by adding an additional overload specifically for null | undefined:

export declare function atom<Value>(initialValue: null | undefined): [Value] extends [Function] ? never : PrimitiveAtom<Value> & WithInitialValue<Value>;
export declare function atom<Value>(read: Read<Value>): Atom<Value>;
export declare function atom<Value>(initialValue: Value): [Value] extends [Function] ? never : PrimitiveAtom<Value> & WithInitialValue<Value>;

A little verbose but it works. I sent a PR.

@dai-shi
Copy link
Member

dai-shi commented Jun 24, 2021

Hmm, I can't reproduce this.

Oh, I guess your strictNullChecks is off?
https://www.typescriptlang.org/tsconfig#strictNullChecks

Proper typing would be not possible without strictNullChecks.

@andrewagain
Copy link
Author

Ah, yes you are correct. This issue only occurs with strictNullChecks=false.

@dai-shi
Copy link
Member

dai-shi commented Jun 24, 2021

Is there any reason you can't enable strictNullChecks. I don't think we can solve it with strictNullChecks=false

#554 isn't necessary with strictNullChecks=true. And, with strictNullChecks=false #554 is not enough (it doesn't infer correctly, and other tests fail).

@andrewagain
Copy link
Author

I do plan to enable strictNullChecks soon but it will be a pretty large effort in my project - hundreds of errors to fix when I turn it on.

strictNullChecks=false is the default with a brand new tsconfig.json from tsc --init so if this goes unfixed, Jotai might seem broken to new users. The only reason I have strictNullChecks=false is because it is the default and I never changed it.

I also have about 10 dependencies that I use types for, and I'm not getting any errors from the other libraries, so I believe this should be possible. Unless Jotai is doing something totally unique here? Perhaps it is doing more overloading than most other libraries?

In any case I'll see if I can convince you with a solution that is better than #544 - but probably not until next week.

@dai-shi
Copy link
Member

dai-shi commented Jun 25, 2021

the default with a brand new tsconfig.json from tsc --init

wow, really... Does it mean strict: false ?

with a solution that is better than #544

If you try yarn test with strict=false, you see some errors. Found 7 errors.
I don't know how easy it is to solve all. Feels impossible.

At the same time, I don't think the current atom types are ideal, so putting your effort here and doing some experiments might reveal something that can improve types even with strict: true.

@andrewagain
Copy link
Author

I double checked and I was not correct. tsc --init actually creates a file that does have strict: true. However, if you have an empty tsconfig.json, it will give you the default which is strict: false. ( https://www.typescriptlang.org/tsconfig#strict )

I ended up changing my project to strict: true so this issue isn't important to me anymore - feel free to close.

@dai-shi
Copy link
Member

dai-shi commented Jul 1, 2021

the default which is strict: false

Yes, I know. This is very unfortunate and annoying.

Closing. Look forward to another issue report!

@alexluong
Copy link

Hi there, is there any way we can re-open this?

I'm hoping we can use jotai but we have strict: false because we're using GraphQL codegen and it's a little tricky to be strict with it.

@dai-shi
Copy link
Member

dai-shi commented Jul 11, 2021

It’s closed because I think it’s impossible to fix in a reasonable way. But I might be wrong. Happy to consider suggestions from anyone.
The workaround for you should be explicit typing. How would that work for you?

@alexluong
Copy link

The workaround for you should be explicit typing.

Can you give an example of what you mean?

@alexluong
Copy link

Just FYI I ended up setting struct: true as well, but I think it would be good to know the workaround if possible! Thanks for being very responsive and I really enjoy using jotai so far!

@dai-shi
Copy link
Member

dai-shi commented Jul 11, 2021

The workaround for you should be explicit typing.

Can you give an example of what you mean?

Sorry, it's not explicit typing, but type assertion.
Like OP noted:

const myAtomConfig = atom<boolean | null>(null) as PrimitiveAtom<
  boolean | null
>;

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 a pull request may close this issue.

3 participants