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.

Sign up for GitHub

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

Temporarily revert useSyncExternalStore changes (#8785). #9393

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Temporarily revert useSyncExternalStore changes (#8785). #9393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 2 additions & 36 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 2 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,8 @@
"peerDependencies": {
"graphql": "^14.0.0 || ^15.0.0 || ^16.0.0",
"graphql-ws": "^5.5.5",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0-beta",
"subscriptions-transport-ws": "^0.9.0 || ^0.11.0",
"use-sync-external-store": "^1.0.0 || ^1.0.0-rc || ^1.0.0-beta"
"react": "^16.8.0 || ^17.0.0",
"subscriptions-transport-ws": "^0.9.0 || ^0.11.0"
},
"peerDependenciesMeta": {
"graphql-ws": {
Expand All @@ -78,9 +77,6 @@
},
"subscriptions-transport-ws": {
"optional": true
},
"use-sync-external-store": {
"optional": true
}
},
"dependencies": {
Expand Down Expand Up @@ -112,7 +108,6 @@
"@types/node": "16.11.19",
"@types/react": "17.0.34",
"@types/react-dom": "17.0.2",
"@types/use-sync-external-store": "^0.0.3",
"acorn": "8.6.0",
"bundlesize": "0.18.1",
"cross-fetch": "3.1.4",
Expand All @@ -138,7 +133,6 @@
"ts-jest": "27.1.2",
"ts-node": "10.4.0",
"typescript": "4.5.2",
"use-sync-external-store": "1.0.0-rc.0",
"wait-for-observables": "1.0.3",
"whatwg-fetch": "3.6.2"
},
Expand Down
18 changes: 12 additions & 6 deletions src/core/ObservableQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,11 @@ export class ObservableQuery<
variablesMustMatch?: boolean,
) {
const last = this.last;
if (last &&
last[key] &&
(!variablesMustMatch || equal(last!.variables, this.variables))) {
if (
last &&
last[key] &&
(!variablesMustMatch || equal(last.variables, this.variables))
) {
return last[key];
}
}
Expand Down Expand Up @@ -326,7 +328,7 @@ export class ObservableQuery<
// (no-cache, network-only, or cache-and-network), override it with
// network-only to force the refetch for this fetchQuery call.
const { fetchPolicy } = this.options;
if (fetchPolicy === 'standby' || fetchPolicy === 'cache-and-network') {
if (fetchPolicy === 'cache-and-network') {
reobserveOptions.fetchPolicy = fetchPolicy;
} else if (fetchPolicy === 'no-cache') {
reobserveOptions.fetchPolicy = 'no-cache';
Expand Down Expand Up @@ -761,8 +763,12 @@ once, rather than every time you call fetchMore.`);
result: ApolloQueryResult<TData>,
variables: TVariables | undefined,
) {
if (this.getLastError() || this.isDifferentFromLastResult(result)) {
this.updateLastResult(result, variables);
const lastError = this.getLastError();
if (lastError || this.isDifferentFromLastResult(result)) {
if (lastError || !result.partial || this.options.returnPartialData) {
this.updateLastResult(result, variables);
}

iterateObserversSafely(this.observers, 'next', result);
}
}
Expand Down
17 changes: 5 additions & 12 deletions src/react/components/__tests__/client/Mutation.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,11 +1021,7 @@ describe('General Mutation testing', () => {
return (
<Mutation mutation={mutation} refetchQueries={refetchQueries}>
{(createTodo: any, resultMutation: any) => (
<Query
query={query}
variables={variables}
notifyOnNetworkStatusChange={true}
>
<Query query={query} variables={variables}>
{(resultQuery: any) => {
try {
if (count === 0) {
Expand All @@ -1051,16 +1047,13 @@ describe('General Mutation testing', () => {
// mutation loading
expect(resultMutation.loading).toBe(true);
} else if (count === 6) {
// mutation still loading???
expect(resultMutation.loading).toBe(true);
} else if (count === 7) {
expect(resultQuery.loading).toBe(true);
// mutation loaded
expect(resultMutation.loading).toBe(false);
} else if (count === 8) {
} else if (count === 7) {
// query refetched
expect(resultQuery.data).toEqual(peopleData3);
expect(resultQuery.loading).toBe(false);
expect(resultMutation.loading).toBe(false);
expect(resultQuery.data).toEqual(peopleData3);
}
count++;
} catch (err) {
Expand All @@ -1081,7 +1074,7 @@ describe('General Mutation testing', () => {
);

waitFor(() => {
expect(count).toBe(9);
expect(count).toEqual(8);
}).then(resolve, reject);
}));

Expand Down
64 changes: 27 additions & 37 deletions src/react/components/__tests__/client/Query.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1229,11 +1229,7 @@ describe('Query component', () => {
const { variables } = this.state;

return (
<AllPeopleQuery
query={query}
variables={variables}
notifyOnNetworkStatusChange={true}
>
<AllPeopleQuery query={query} variables={variables}>
{(result: any) => {
try {
switch (count) {
Expand Down Expand Up @@ -1721,40 +1717,35 @@ describe('Query component', () => {
<AllPeopleQuery
query={query}
variables={variables}
notifyOnNetworkStatusChange={true}
onCompleted={this.onCompleted}
>
{({ loading, data }: any) => {
try {
switch (renderCount) {
case 0:
expect(loading).toBe(true);
break;
case 1:
case 2:
expect(loading).toBe(false);
expect(data).toEqual(data1);
break;
case 3:
expect(loading).toBe(true);
break;
case 4:
expect(loading).toBe(false);
expect(data).toEqual(data2);
setTimeout(() => {
this.setState({ variables: { first: 1 } });
});
case 5:
expect(loading).toBe(false);
expect(data).toEqual(data2);
break;
case 6:
expect(loading).toBe(false);
expect(data).toEqual(data1);
break;
}
} catch (err) {
reject(err);
switch (renderCount) {
case 0:
expect(loading).toBe(true);
break;
case 1:
case 2:
expect(loading).toBe(false);
expect(data).toEqual(data1);
break;
case 3:
expect(loading).toBe(true);
break;
case 4:
expect(loading).toBe(false);
expect(data).toEqual(data2);
setTimeout(() => {
this.setState({ variables: { first: 1 } });
});
case 5:
expect(loading).toBe(false);
expect(data).toEqual(data2);
break;
case 6:
expect(loading).toBe(false);
expect(data).toEqual(data1);
break;
}
renderCount += 1;
return null;
Expand All @@ -1771,7 +1762,6 @@ describe('Query component', () => {
);

waitFor(() => {
expect(renderCount).toBe(7);
expect(onCompletedCallCount).toBe(3);
}).then(resolve, reject);
});
Expand Down
1 change: 0 additions & 1 deletion src/react/components/__tests__/ssr/getDataFromTree.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/** @jest-environment node */
import React from 'react';
import gql from 'graphql-tag';
import { DocumentNode } from 'graphql';
Expand Down
1 change: 0 additions & 1 deletion src/react/components/__tests__/ssr/server.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/** @jest-environment node */
import React from 'react';
import {
print,
Expand Down
17 changes: 6 additions & 11 deletions src/react/hoc/__tests__/queries/errors.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,7 @@ describe('[queries] errors', () => {
let iteration = 0;
let done = false;
const ErrorContainer = withState('var', 'setVar', 1)(
graphql<Props, Data, Vars>(
query,
{ options: { notifyOnNetworkStatusChange: true }},
)(
graphql<Props, Data, Vars>(query)(
class extends React.Component<ChildProps<Props, Data, Vars>> {
componentDidUpdate() {
const { props } = this;
Expand All @@ -237,7 +234,7 @@ describe('[queries] errors', () => {
);
} else if (iteration === 3) {
// variables have changed, wee are loading again but also have data
expect(props.data!.loading).toBe(true);
expect(props.data!.loading).toBeTruthy();
} else if (iteration === 4) {
// the second request had an error!
expect(props.data!.error).toBeTruthy();
Expand All @@ -259,8 +256,8 @@ describe('[queries] errors', () => {
render() {
return null;
}
},
),
}
)
);

render(
Expand Down Expand Up @@ -473,11 +470,9 @@ describe('[queries] errors', () => {
});
break;
case 3:
// Second render was added by useSyncExternalStore changes...
case 4:
expect(props.data!.loading).toBeTruthy();
break;
case 5:
case 4:
expect(props.data!.loading).toBeFalsy();
expect(props.data!.error).toBeFalsy();
expect(props.data!.allPeople).toEqual(
Expand All @@ -504,7 +499,7 @@ describe('[queries] errors', () => {
</ApolloProvider>
);

waitFor(() => expect(count).toBe(6)).then(resolve, reject);
waitFor(() => expect(count).toBe(5)).then(resolve, reject);
});

itAsync('does not throw/console.err an error after a component that received a network error is unmounted', (resolve, reject) => {
Expand Down
Loading