Closed andyrichardson closed 4 years ago
The console.warn and throw Error can be replaced with our warning and invariant helpers and there's probably some other stuff that can be reused 💯
Then we do need to add codes for them
Types look pretty vague on Invariant (i.e. what is code?) - let me know if I'm doing this wrong.
Also, would be nice to do
if (condition)
throw invariant(...args)
}
someOtherCode();
rather than
if (condition) {
invariant(...args); // Intention to throw isn't clear to newcomers
}
someOtherCode();
Let's do this! 💯 Great work Andy!
We still need to add the export though
Done :+1:
Looked at typing the errors but I'm not sure numbers is the way to do this.
String literals would make more sense:
Numbers
// Types
type ErrorCode = InvalidGraphQLDocumentCode | InvalidCacheCallCode;
type InvalidGraphQLDocumentCode = 1;
type InvalidCacheCallCode = 2;
// Usage
warn(msg, 1); // 1 ???
// User
if (err.code === 1) // What does this mean?
Literals
// Types
type ErrorCode = "INVALID_GRAPHQL_DOCUMENT" | "INVALID_CACHE_CALL";
// Usage
warn(msg, "INVALID_GRAPHQL_DOCUMENT"); // makes more sense
// User
if (err.code === "INVALID_GRAPHQL_DOCUMENT")
Breaking change but removes the confusion when catching errors (most users won't be catching anyhow)
@andyrichardson They're not any typed/relevant type information, but they're just numeric identifiers that we add to our help.md
to add more information on how a warning/error can be resolved and why it happened 🆘 https://github.com/FormidableLabs/urql-exchange-graphcache/blob/master/docs/help.md
So typically just choose the next highest number in that doc and add a new section and copy that info over 😅 It's not linted (yet)
Re: Throw vs invariant; it's a pattern we're basically adopting from React and other libraries. invariant
/ assert
are typically good uses of asserting some type information or other condition. That being said, there are some places where we need to clean up some invariant(false, ...)
uses. But I have a branch for that that I just haven't merged yet. https://github.com/FormidableLabs/urql-exchange-graphcache/tree/chore/upgrade-ts
Happy to also open a PR after we merge this that does all of this at the same time
They're not any typed/relevant type information, but they're just numeric identifiers that we add to our help.md to add more information on how a warning/error can be resolved and why it happened
@kitten yeah I totally get what the numbers are for. Literals would do the same job, just a better DX.
Going back on topic, what's left for this PR? The only thing I'm aware of is the number being thrown from the invariant call. Can't see anyone looking to wrap the exchange in a try/catch so I wouldn't put too much thought into it.
For discussion, see https://github.com/FormidableLabs/urql/pull/471