Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid warning about clobbering scalar fields. #7075

Merged
merged 3 commits into from
Sep 25, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Sep 25, 2020

While it is possible to write a custom merge function that handles updates for scalar field values, we should treat scalar data as opaque by default, without displaying Cache data may be lost... warnings (#6372) that nudge the developer to write an unnecessary merge function.

In this context, scalar data means any subtree of a GraphQL result object for which the corresponding field in the query does not have a selection set. In particular, JSON data may look like any other GraphQL object, but it should be treated as an opaque value if the query does not apply a nested selection set to the field that holds the JSON data.

Should fix #7071.

While it is possible to write a field merge function that handles updates
for scalar field values, we should treat scalar data as opaque by default,
without displaying "Cache data may be lost..." warnings that nudge the
developer to write an unnecessary merge function.

None: because this code is inside of a process.env.NODE_ENV block, it will
not be executed or bundled in production.

Should fix #7071.
@benjamn benjamn force-pushed the 7071-no-warning-for-scalar-field-update branch from db67108 to a4f9ca5 Compare September 25, 2020 17:20
Comment on lines 286 to +294
if (process.env.NODE_ENV !== "production") {
const hasSelectionSet = (storeFieldName: string) =>
fieldsWithSelectionSets.has(fieldNameFromStoreName(storeFieldName));
const fieldsWithSelectionSets = new Set<string>();
workSet.forEach(selection => {
if (isField(selection) && selection.selectionSet) {
fieldsWithSelectionSets.add(selection.name.value);
}
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this code is inside of a process.env.NODE_ENV !== "production" block, it will not be executed or bundled in production.

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff @benjamn - thanks!

@benjamn benjamn merged commit 3caa496 into release-3.3 Sep 25, 2020
@benjamn benjamn deleted the 7071-no-warning-for-scalar-field-update branch September 25, 2020 19:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants