Fetch policy constants #1488
Fetch policy constants #1488
Conversation
| this.options, | ||
| this.queryId, | ||
| ); | ||
| this.startPolling(this.options.pollInterval); |
There was a problem hiding this comment.
Seemed a case of repeated logic so I opted to call the existing method instead.
There was a problem hiding this comment.
Thanks @NeoPhi!
I think this change is only useful for JS users. As such it might be better if we limit it to the one type definitions file you created and just export it from there for the benefit of JS users. The reasoning is that using typescript and a good editor makes this unnecessary, because it will propose the different options to you. Even if you're using Apollo Client in Javascript, I suggest you try out VScode, because it will suggest / auto-complete the fetchPolicy strings for you!
That said, I'm not completely opposed to using constants in this way, even though it's not idiomatic typescript. But in that case we'd definitely have to use those constants when defining the type, otherwise it kind of defeats the whole point of using constants.
What do you think @stubailo?
| /** | ||
| * fetchPolicy determines where the client may return a result from. | ||
| */ | ||
| export type FetchPolicy = 'cache-first' | 'cache-and-network' | 'network-only' | 'cache-only'; |
There was a problem hiding this comment.
Constants are only useful for Javascript users. In Typescript they're not necessary at all. If we do use constants everywhere, this is the one line we definitely shouldn't skip!
There was a problem hiding this comment.
Wasn't sure of a way in TypeScript to define a type using constants. Instead I defined the constants to be of the FetchPolicy type.
There was a problem hiding this comment.
This sounds like a job for a TypeScript Enum - I think you can have TS compile that into a dictionary with values.
| export const CACHE_FIRST: FetchPolicy = 'cache-first'; | ||
|
|
||
| /** | ||
| * returns result from cache first (if it exists), then return network result once it's available. |
|
I'm wondering what the desired benefit of constants is in this case over strings - is it because there's a worry we might later need to change the underlying values of these constants? If we are going to keep the same string values forever, it seems like we don't get a lot of value out of wrapping them in constants. |
|
I couldn't find a way to do this cleanly in TypeScript with easy support for vanilla JS. |
TODO:
What to confirm the constant naming and approach for exporting in a namespace before updating any documentation.