Fix local state updates for GraphQL Query Variables #7205
Conversation
|
|
|
|
| if (operationName) { | ||
| content.operationName = operationName; | ||
| // The below items are optional; should be set to undefined if present and empty | ||
| if (operationName !== undefined) { |
There was a problem hiding this comment.
We may also check if it is null or operationName.length would fail below.
| content.operationName = operationName; | ||
| // The below items are optional; should be set to undefined if present and empty | ||
| if (operationName !== undefined) { | ||
| content.operationName = operationName.length ? operationName : undefined; |
There was a problem hiding this comment.
We might check it with Array.isArray
There was a problem hiding this comment.
I don't think Array.isArray will work for validating a string which would be expected for operationName, but we could cover both checks by asserting that the operationName and variables params are strings. I'd argue that null should never be given as an input anyway and it would be caught by the type checker, but I don't think there's harm in making the check anyway.
Pushed an updated approach
There was a problem hiding this comment.
Not sure what your thoughts are on including the (value as unknown) instanceof String check is. I included it for completeness but I'm guessing there's no usage of the String wrapper object anywhere in the codebase.
There was a problem hiding this comment.
My bad, it's fine to include (value as unknown) instanceof String although typeof should be good enough for most of cases.
filfreire
force-pushed
the
fix_query_variables_not_persisting_updates
branch
from
010de79
to
55cd65e
Compare
…om:MKHokinson/insomnia into fix_query_variables_not_persisting_updates
| content.operationName = operationName; | ||
| // The below items are optional; should be set to undefined if present and empty | ||
| if (operationName !== undefined) { | ||
| content.operationName = operationName.length ? operationName : undefined; |
There was a problem hiding this comment.
My bad, it's fine to include (value as unknown) instanceof String although typeof should be good enough for most of cases.
Closes #7204