add warning around missing __typename in fragment payload #413
Conversation
| */ | ||
| getAddTypename() { | ||
| return this._addTypename; | ||
| } |
There was a problem hiding this comment.
Rather than converting this into a getter, let's make the property public
src/operations/SnapshotEditor.ts
Outdated
| @@ -223,6 +223,13 @@ export class SnapshotEditor { | |||
| if (payload && payloadName in payload) { | |||
| warnings.push(`Encountered undefined at ${[...prefixPath, ...path].join('.')}. Treating as null`); | |||
| } | |||
|
|
|||
| if (this._context.getAddTypename() && payloadName === '__typename') { | |||
| const existingTypenameValue = deepGet(this._getNodeData(containerId), path); | |||
There was a problem hiding this comment.
You can skip this check - it's even more of a problem for cases where typename is missing (for some consumers of hermes, __typename is part of object identity)
There was a problem hiding this comment.
done. For unit testing, how to test? I didn't see an existing example checking tracer warnings. I see tests for catching InvalidPayloadError. By the way, should this be an InvalidPayloadError instead of a warning? That would be more visible to consumers.
There was a problem hiding this comment.
Yeah, failing hard here would be 👍
re: unit testing the tracer, we typically fall back to testing the console output, rather than the tracer itself (since that's more stable, and these messages are likely to change). But it's also not a very proven pattern; injecting the tracer might be better.
Example test: https://github.com/convoyinc/apollo-cache-hermes/blob/master/test/unit/Cache/transactions.ts#L53-L62
finnigantime
force-pushed
the
finnigan/412-emit-warning
branch
from
2af6b81
to
7059b38
Compare
finnigantime
force-pushed
the
finnigan/412-emit-warning
branch
from
5b7e59d
to
1faf2f8
Compare
✔ Write a summary
Add warning around missing __typename in fragment payload. See #412. This is my attempt to add a warning around this condition. Note that the warning is only emitted if verbose logging is enabled so I'm not sure how helpful it'll be.
✔ Tests!
N/A
✔ Bump the version if needed
N/A