[typescript] Issue with generated .d.ts for withClientState

[Phoenixmatrix]

I'm not 100% sure if this is just an issue with my setup, but with strict null and no implicit any in typescript, compilation fails with the error: Type 'ClientStateConfig | undefined' has no property 'resolvers' and no string index signature unless I skip lib checks.

Looking at the type definition, I see:

export declare const withClientState: ({resolvers, defaults, cache}?: ClientStateConfig) => ApolloLink;

This works fine on the TS playground with the same TS version I have locally (with the same options), so I'm a little confused. However looking closer, I'm wondering why the object argument for ClientStateConfig is nullable.

When I look at the actual definition, it is not.

Locally changing the definition to:
export declare const withClientState: ({resolvers, defaults, cache}: ClientStateConfig) => ApolloLink; (notice the removal of the ?) fixes the issue, but I'm unfamiliar with how the d.ts are generated (I'm no TS expert) and why there's a mismatch.

[peggyrayzis]

Hmm I'm not sure why the .d.ts file looks like that. Technically, you can just call withClientState() and pass in nothing if you only wanted to use the link for stripping out the @client directive. If you pass in nothing, resolvers will be set to an empty object.

@fbartho have you experienced this issue? I know you're also using Typescript.

[Phoenixmatrix]

Yeah, that makes sense if the config is optional. The issue is mostly with the destructing with strict null.

Actually, I see now that in the implementation, there's a default provided (which makes sense: without a default, the destructuring would fail when passing nothing). So that's why the type definition has the type as nullable.

So the implementation is valid and this is just yet another TS + strict mode weirdness. It's a blocker with that setup though. Because it's an export const, I can't even make my own .d.ts and override the type definition (Assuming the problem is not something I'm doing, which is very possible!)

[Phoenixmatrix]

Digging a bit further, this seems to be an instance of this typescript bug at first glance.

However, the bug was fixed long ago in this PR, and even using the latest typescript version I get the same result.

[fbartho]

I'm not running TypeScript in strict mode due to it exploding on several of the react-native and other node_modules that I'm dependent on. I'd love to be able to be strict, but there's lots of slightly wrong typings out there. That probably explains why I wasn't seeing it @peggyrayzis.

[peggyrayzis]

It might be worth opening another issue about this bug on the Typescript repo @Phoenixmatrix. At the very least, maybe they can give us a suggestion on how to properly type it. 😀

[Phoenixmatrix]

Yeah, I should do that.

Right now, a pretty crappy suggestion (but one that has been used by some other projects linked on that existing issue) is to do the destructuring inside of the function instead of in the argument.

It's kind of meh that you'd have to change perfectly valid code to accommodate this case, but it would be simple and would unblock the strict mode use case.

I suppose the first question here is: is supporting strict mode TypeScript within the scope of this project? (even if we open the issue again, it probably will be a while before if its fixed and this might not be the last time a workaround has to happen... though right now its the only one Ive hit)

[fbartho]

In my opinion @Phoenixmatrix, we're perfectly happy accepting patches that support strict-mode TypeScript.

@peggyrayzis's wider experience here could be valuable in answering this: does Apollo care about strict mode? I don't know what we'd be signing up for in terms of pain, as I'm very new to TypeScript.

[peggyrayzis]

Supporting strict mode is not at the top of our priority list, but I'd gladly accept any PRs from the community with changes to support strict mode.

[Phoenixmatrix]

Looks like microsoft/TypeScript/issues/10078 has already been opened (and has been bumped from milestone to milestone) for a while, so in the short term, it seems the best solution is to just work around it.