From ddc9f727b9929b99f101b92fd4229ec2e7d1eedc Mon Sep 17 00:00:00 2001 From: Vicary A Date: Sat, 5 Aug 2023 22:09:08 +0800 Subject: [PATCH] fix(react): reduce over-fetching between renders #1594 --- examples/gnt/app/MyComponent.tsx | 43 ------------ examples/gnt/app/{ => components}/Avatar.tsx | 0 examples/gnt/app/{ => components}/Card.tsx | 0 .../gnt/app/components/CscCharacterSearch.tsx | 68 +++++++++++++++++++ .../app/components/CscCharactersSearch.tsx | 66 ++++++++++++++++++ .../RscCharacter.tsx} | 4 +- .../gnt/app/{ => components}/Skeleton.tsx | 0 .../gnt/app/{ => components}/SmallText.tsx | 0 examples/gnt/app/{ => components}/Text.tsx | 0 examples/gnt/app/page.tsx | 10 ++- examples/gnt/gqty/index.ts | 7 +- examples/gnt/package.json | 3 +- internal/test-utils/package.json | 2 +- packages/cli/package.json | 4 +- packages/gqty/package.json | 6 +- packages/gqty/src/Client/batching.ts | 8 +-- packages/gqty/src/Client/debugger.ts | 9 +-- packages/gqty/src/Client/resolveSelections.ts | 2 +- packages/gqty/src/Client/resolvers.ts | 10 +-- packages/logger/package.json | 4 +- packages/logger/src/index.ts | 16 +++-- packages/logger/test/index.test.ts | 10 ++- packages/logger/test/tsconfig.json | 3 +- packages/react/package.json | 4 +- packages/react/src/query/useQuery.ts | 8 ++- packages/subscriptions/package.json | 2 +- 26 files changed, 193 insertions(+), 96 deletions(-) delete mode 100644 examples/gnt/app/MyComponent.tsx rename examples/gnt/app/{ => components}/Avatar.tsx (100%) rename examples/gnt/app/{ => components}/Card.tsx (100%) create mode 100644 examples/gnt/app/components/CscCharacterSearch.tsx create mode 100644 examples/gnt/app/components/CscCharactersSearch.tsx rename examples/gnt/app/{Character.tsx => components/RscCharacter.tsx} (68%) rename examples/gnt/app/{ => components}/Skeleton.tsx (100%) rename examples/gnt/app/{ => components}/SmallText.tsx (100%) rename examples/gnt/app/{ => components}/Text.tsx (100%) diff --git a/examples/gnt/app/MyComponent.tsx b/examples/gnt/app/MyComponent.tsx deleted file mode 100644 index f242b9c93..000000000 --- a/examples/gnt/app/MyComponent.tsx +++ /dev/null @@ -1,43 +0,0 @@ -'use client'; - -import { useDeferredValue, useState, type FunctionComponent } from 'react'; -import { useQuery } from '~/gqty/react'; -import Avatar from './Avatar'; -import Card from './Card'; -import SmallText from './SmallText'; -import Text from './Text'; - -export type Props = {}; - -const MyComponent: FunctionComponent = () => { - const [name, setName] = useState('Rick'); - const deferredName = useDeferredValue(name); - const query = useQuery(); - - return ( - <> - setName(e.target.value)} - className="border border-gray-300 rounded-md px-3 py-2 w-full text-black" - /> - - {query - .characters({ filter: { name: deferredName } }) - ?.results?.map((character) => ( - - - -
- {character?.name} - {character?.species} - {character?.origin?.name} -
-
- ))} - - ); -}; - -export default MyComponent; diff --git a/examples/gnt/app/Avatar.tsx b/examples/gnt/app/components/Avatar.tsx similarity index 100% rename from examples/gnt/app/Avatar.tsx rename to examples/gnt/app/components/Avatar.tsx diff --git a/examples/gnt/app/Card.tsx b/examples/gnt/app/components/Card.tsx similarity index 100% rename from examples/gnt/app/Card.tsx rename to examples/gnt/app/components/Card.tsx diff --git a/examples/gnt/app/components/CscCharacterSearch.tsx b/examples/gnt/app/components/CscCharacterSearch.tsx new file mode 100644 index 000000000..38e2612db --- /dev/null +++ b/examples/gnt/app/components/CscCharacterSearch.tsx @@ -0,0 +1,68 @@ +'use client'; + +import type { Variables } from 'gqty'; +import { useState, type FunctionComponent } from 'react'; +import Button from '~/components/tailwindui/Button'; +import type { Query } from '~/gqty'; +import { useQuery } from '~/gqty/react'; +import Avatar from './Avatar'; +import Card from './Card'; +import SmallText from './SmallText'; +import Text from './Text'; + +export type Props = {}; + +const Component: FunctionComponent = () => { + const [searchValue, setSearchValue] = useState(); + + return ( + <> + + {searchValue && } + + ); +}; + +const SelectBox: FunctionComponent<{ + onChange?: (value: string) => void; +}> = ({ onChange }) => { + const [value, setValue] = useState(); + + return ( +
+ setValue(e.target.value)} + className="border border-gray-300 rounded-md px-3 py-2 w-full text-black" + /> + +
+ ); +}; + +const Character: FunctionComponent> = (props) => { + const character = useQuery().character(props); + + return ( + + + +
+ {character?.name} + {character?.species} + {character?.origin?.name} +
+
+ ); +}; + +export default Component; diff --git a/examples/gnt/app/components/CscCharactersSearch.tsx b/examples/gnt/app/components/CscCharactersSearch.tsx new file mode 100644 index 000000000..4d7f42276 --- /dev/null +++ b/examples/gnt/app/components/CscCharactersSearch.tsx @@ -0,0 +1,66 @@ +'use client'; + +import type { Variables } from 'gqty'; +import { useState, type FunctionComponent } from 'react'; +import Button from '~/components/tailwindui/Button'; +import type { Query } from '~/gqty'; +import { useQuery } from '~/gqty/react'; +import Avatar from './Avatar'; +import Card from './Card'; +import SmallText from './SmallText'; +import Text from './Text'; + +export type Props = {}; + +const Component: FunctionComponent = () => { + const [searchValue, setSearchValue] = useState(); + + return ( + <> + + + + ); +}; + +const SearchBox: FunctionComponent<{ + onChange?: (value: string) => void; +}> = ({ onChange }) => { + const [inputName, setInputName] = useState(''); + + return ( +
+ setInputName(e.target.value)} + className="border border-gray-300 rounded-md px-3 py-2 w-full text-black" + /> + +
+ ); +}; + +const Characters: FunctionComponent> = ( + props +) => { + const query = useQuery(); + + return ( + <> + {query.characters(props)?.results?.map((character) => ( + + + +
+ {character?.name} + {character?.species} + {character?.origin?.name} +
+
+ ))} + + ); +}; + +export default Component; diff --git a/examples/gnt/app/Character.tsx b/examples/gnt/app/components/RscCharacter.tsx similarity index 68% rename from examples/gnt/app/Character.tsx rename to examples/gnt/app/components/RscCharacter.tsx index acd56d9b4..86ec0c635 100644 --- a/examples/gnt/app/Character.tsx +++ b/examples/gnt/app/components/RscCharacter.tsx @@ -1,7 +1,7 @@ -import { resolve, type Query } from '../gqty'; +import { resolve, type Query } from '~/gqty'; /** RSC */ -export default async function Character({ +export default async function RscCharacter({ id, }: Parameters[0]) { const data = await resolve(({ query }) => { diff --git a/examples/gnt/app/Skeleton.tsx b/examples/gnt/app/components/Skeleton.tsx similarity index 100% rename from examples/gnt/app/Skeleton.tsx rename to examples/gnt/app/components/Skeleton.tsx diff --git a/examples/gnt/app/SmallText.tsx b/examples/gnt/app/components/SmallText.tsx similarity index 100% rename from examples/gnt/app/SmallText.tsx rename to examples/gnt/app/components/SmallText.tsx diff --git a/examples/gnt/app/Text.tsx b/examples/gnt/app/components/Text.tsx similarity index 100% rename from examples/gnt/app/Text.tsx rename to examples/gnt/app/components/Text.tsx diff --git a/examples/gnt/app/page.tsx b/examples/gnt/app/page.tsx index 4e7cd2128..c0e2ad37f 100644 --- a/examples/gnt/app/page.tsx +++ b/examples/gnt/app/page.tsx @@ -1,6 +1,10 @@ import { Suspense } from 'react'; -import Character from './Character'; -import MyComponent from './MyComponent'; +import CharacterSearch from './components/CscCharacterSearch'; +import CharactersSearch from './components/CscCharactersSearch'; +import Character from './components/RscCharacter'; + +CharacterSearch; +CharactersSearch; export default function Home() { return ( @@ -8,7 +12,7 @@ export default function Home() { {/* CSR test */} Loading...}> - + {/* RSC test */} diff --git a/examples/gnt/gqty/index.ts b/examples/gnt/gqty/index.ts index 6d86252ef..c5c4bb911 100644 --- a/examples/gnt/gqty/index.ts +++ b/examples/gnt/gqty/index.ts @@ -2,6 +2,7 @@ * GQty: You can safely modify this file based on your needs. */ +import { createLogger } from '@gqty/logger'; import { Cache, GQtyError, createClient, type QueryFetcher } from 'gqty'; import { generatedSchema, @@ -13,8 +14,6 @@ const queryFetcher: QueryFetcher = async function ( { query, variables, operationName, extensions }, fetchOptions ) { - console.debug({ query, variables, operationName, ...extensions }); - // Modify "https://rickandmortyapi.com/graphql" if needed const response = await fetch('https://rickandmortyapi.com/graphql', { method: 'POST', @@ -32,7 +31,7 @@ const queryFetcher: QueryFetcher = async function ( if (response.status >= 400) { throw new GQtyError( - `GraphQL endpoint responded with HTTP ${response.status}: ${response.statusText}.` + `GraphQL endpoint responded with HTTP status ${response.status}.` ); } @@ -71,6 +70,8 @@ export const client = createClient({ }, }); +createLogger(client).start(); + // Core functions export const { resolve, subscribe, schema } = client; diff --git a/examples/gnt/package.json b/examples/gnt/package.json index 507ac6928..6e81bd2cb 100644 --- a/examples/gnt/package.json +++ b/examples/gnt/package.json @@ -27,6 +27,7 @@ "eslint-config-next": "13.3.0", "postcss": "8.4.21", "tailwindcss": "^3.3.3", - "typescript": "^5.0.4" + "typescript": "^5.0.4", + "utf-8-validate": "^5.0.2" } } diff --git a/internal/test-utils/package.json b/internal/test-utils/package.json index 146a3e303..0658ed301 100644 --- a/internal/test-utils/package.json +++ b/internal/test-utils/package.json @@ -28,7 +28,7 @@ "build": "bob-ts -i src -f interop", "prepare": "pnpm build", "start": "nodemon --exec \"concurrently pnpm:build tsc\" -w src/index.ts", - "test": "NODE_OPTIONS=--experimental-vm-modules jest --config local.jest.config.js" + "test": "jest --config local.jest.config.js" }, "dependencies": { "@graphql-ez/fastify": "^0.12.0", diff --git a/packages/cli/package.json b/packages/cli/package.json index f662107e4..ada5a2df2 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -38,8 +38,8 @@ "build": "bob-tsm build.ts", "prepare": "bob-tsm build.ts", "postpublish": "gh-release", - "test": "NODE_OPTIONS=--experimental-vm-modules jest", - "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch", + "test": "jest", + "test:watch": "jest --watch", "test:watch-coverage": "rimraf coverage && mkdirp coverage/lcov-report && concurrently --raw \"jest --watchAll\" \"serve -l 8787 coverage/lcov-report\" \"wait-on tcp:8787 coverage/lcov-report/index.html && open-cli http://localhost:8787\"" }, "dependencies": { diff --git a/packages/gqty/package.json b/packages/gqty/package.json index f25c89a6e..47df2d62c 100644 --- a/packages/gqty/package.json +++ b/packages/gqty/package.json @@ -38,9 +38,9 @@ "postpublish": "gh-release", "size": "size-limit", "start": "bob-esbuild watch", - "test": "NODE_OPTIONS=--experimental-vm-modules jest", - "test:specific": "NODE_OPTIONS=--experimental-vm-modules jest test/interfaces-unions.test.ts --watch --no-coverage -u", - "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch", + "test": "jest", + "test:specific": "jest test/interfaces-unions.test.ts --watch --no-coverage -u", + "test:watch": "jest --watch", "test:watch-coverage": "rimraf coverage && mkdirp coverage/lcov-report && concurrently --raw \"jest --watchAll\" \"serve -l 8787 coverage/lcov-report\" \"wait-on tcp:8787 coverage/lcov-report/index.html && open-cli http://localhost:8787\"" }, "dependencies": { diff --git a/packages/gqty/src/Client/batching.ts b/packages/gqty/src/Client/batching.ts index c5dc64079..9317027e2 100644 --- a/packages/gqty/src/Client/batching.ts +++ b/packages/gqty/src/Client/batching.ts @@ -6,7 +6,7 @@ const pendingSelections = new Map>>>(); export const addSelections = ( cache: Cache, key: string, - value: Set + selections: Set ) => { if (!pendingSelections.has(cache)) { pendingSelections.set(cache, new Map()); @@ -18,19 +18,19 @@ export const addSelections = ( selectionsByKey.set(key, new Set()); } - return selectionsByKey.get(key)!.add(value); + return selectionsByKey.get(key)!.add(selections); }; export const getSelectionsSet = (cache: Cache, key: string) => pendingSelections.get(cache)?.get(key); -export const delSelectionsSet = (cache: Cache, key: string) => +export const delSelectionSet = (cache: Cache, key: string) => pendingSelections.get(cache)?.delete(key) ?? false; export const popSelectionsSet = (cache: Cache, key: string) => { const result = getSelectionsSet(cache, key); - delSelectionsSet(cache, key); + delSelectionSet(cache, key); return result; }; diff --git a/packages/gqty/src/Client/debugger.ts b/packages/gqty/src/Client/debugger.ts index da6bf0e49..0221f00f1 100644 --- a/packages/gqty/src/Client/debugger.ts +++ b/packages/gqty/src/Client/debugger.ts @@ -14,19 +14,20 @@ export type DebugEvent = { export type DebugEventListener = (event: DebugEvent) => void; export type Debugger = { - dispatch: (event: DebugEvent) => void; + dispatch: (event: DebugEvent) => Promise; /** Returns an unsubscribe function */ subscribe: (listener: DebugEventListener) => () => void; }; -export const createDebugger = () => { +export const createDebugger = (): Debugger => { const subs = new Set(); return { - dispatch: (event: DebugEvent) => { - subs.forEach((sub) => sub(event)); + dispatch: async (event: DebugEvent) => { + await Promise.all([...subs].map((sub) => sub(event))); }, + subscribe: (listener: DebugEventListener) => { subs.add(listener); return () => subs.delete(listener); diff --git a/packages/gqty/src/Client/resolveSelections.ts b/packages/gqty/src/Client/resolveSelections.ts index 4077737ee..d5bbbbf19 100644 --- a/packages/gqty/src/Client/resolveSelections.ts +++ b/packages/gqty/src/Client/resolveSelections.ts @@ -80,7 +80,7 @@ export const fetchSelections = < } // TODO: Defer logging until after cache update - debug?.dispatch({ + await debug?.dispatch({ cache, request: queryPayload, result, diff --git a/packages/gqty/src/Client/resolvers.ts b/packages/gqty/src/Client/resolvers.ts index eda76635f..f1aec928f 100644 --- a/packages/gqty/src/Client/resolvers.ts +++ b/packages/gqty/src/Client/resolvers.ts @@ -4,7 +4,7 @@ import { type Cache } from '../Cache'; import { type GQtyError, type RetryOptions } from '../Error'; import { type ScalarsEnumsHash, type Schema } from '../Schema'; import { type Selection } from '../Selection'; -import { addSelections, delSelectionsSet, getSelectionsSet } from './batching'; +import { addSelections, delSelectionSet, getSelectionsSet } from './batching'; import { createContext, type SchemaContext } from './context'; import { type Debugger } from './debugger'; import { @@ -251,17 +251,13 @@ export const createResolvers = ({ pendingQueries.delete(pendingSelections); - delSelectionsSet(clientCache, selectionsCacheKey); + delSelectionSet(clientCache, selectionsCacheKey); return fetchSelections(selections, { cache: context.cache, debugger: debug, extensions, - fetchOptions: { - ...fetchOptions, - cachePolicy, - retryPolicy, - }, + fetchOptions: { ...fetchOptions, cachePolicy, retryPolicy }, operationName, }).then((results) => { updateCaches( diff --git a/packages/logger/package.json b/packages/logger/package.json index 6bc616327..e8ba08a64 100644 --- a/packages/logger/package.json +++ b/packages/logger/package.json @@ -37,8 +37,8 @@ "postpublish": "gh-release", "size": "size-limit", "start": "bob-esbuild watch", - "test": "NODE_OPTIONS=--experimental-vm-modules jest", - "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" + "test": "jest", + "test:watch": "jest --watch" }, "dependencies": { "prettier": "^3.0.1" diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 9464793c8..befd93cd2 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -1,20 +1,21 @@ import type { DebugEvent, GQtyClient } from 'gqty'; -import prettierBabel from 'prettier/plugins/babel'; -import prettierGraphQL from 'prettier/plugins/graphql'; -import prettier from 'prettier/standalone'; +import * as prettierBabel from 'prettier/plugins/babel'; +import * as prettierEstree from 'prettier/plugins/estree'; +import * as prettierGraphQL from 'prettier/plugins/graphql'; +import { format as prettierFormat } from 'prettier/standalone'; import { serializeError } from './serializeError'; async function parseGraphQL(query: string) { - return await prettier.format(query, { + return await prettierFormat(query, { parser: 'graphql', plugins: [prettierBabel, prettierGraphQL], }); } async function parseJSON(value: unknown) { - return await prettier.format(JSON.stringify(value), { + return await prettierFormat(JSON.stringify(value), { parser: 'json', - plugins: [prettierBabel], + plugins: [prettierBabel, prettierEstree], }); } @@ -166,8 +167,9 @@ export function createLogger( ...format(['Cache snapshot', headerStyles]), await stringifyJSONIfEnabled(cache?.toJSON()) ); - console.groupEnd(); } + + console.groupEnd(); } /** diff --git a/packages/logger/test/index.test.ts b/packages/logger/test/index.test.ts index ba2a940ac..9423949e5 100644 --- a/packages/logger/test/index.test.ts +++ b/packages/logger/test/index.test.ts @@ -70,9 +70,9 @@ describe('logger', () => { const spyError = jest.spyOn(console, 'error').mockImplementation(); try { - const dataPromise = gqtyClient.resolved(() => { - return gqtyClient.query.hello({ hello: 'hello' }); - }); + const dataPromise = gqtyClient.resolve(({ query }) => + query.hello({ hello: 'hello' }) + ); const data = await dataPromise; @@ -84,9 +84,7 @@ describe('logger', () => { expect(data).toBe('hello world'); - const errorPromise = gqtyClient.resolved(() => { - return gqtyClient.query.throw; - }); + const errorPromise = gqtyClient.resolve(({ query }) => query.throw); await errorPromise.catch(() => {}); diff --git a/packages/logger/test/tsconfig.json b/packages/logger/test/tsconfig.json index a005c732f..b5f92e578 100644 --- a/packages/logger/test/tsconfig.json +++ b/packages/logger/test/tsconfig.json @@ -5,8 +5,7 @@ "esModuleInterop": true, "jsx": "react", "noEmit": true, - "strict": true, - "target": "esnext" + "strict": true }, "exclude": [], "include": ["**/*.ts", "**/*.tsx"] diff --git a/packages/react/package.json b/packages/react/package.json index c1b1704ae..7247dbe05 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -39,8 +39,8 @@ "size": "size-limit", "start": "bob-esbuild watch", "start:with:example": "bob-esbuild watch --onSuccess \"pnpm -r --filter react-example dev\"", - "test": "NODE_OPTIONS=--experimental-vm-modules jest", - "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" + "test": "jest", + "test:watch": "jest --watch" }, "eslintConfig": { "env": { diff --git a/packages/react/src/query/useQuery.ts b/packages/react/src/query/useQuery.ts index 03befc40f..64db8c0bc 100644 --- a/packages/react/src/query/useQuery.ts +++ b/packages/react/src/query/useQuery.ts @@ -184,6 +184,12 @@ export const createUseQuery = ( if (state.promise && !context.hasCacheHit) throw state.promise; } + // Reset selections to prevent overfetching, but do it only when the + // previous render is not triggered by a successful fetch. + if (context.shouldFetch === false) { + selections.clear(); + } + useEffect( () => context.cache.subscribe( @@ -250,8 +256,6 @@ export const createUseQuery = ( context.notifyCacheUpdate = cachePolicy !== 'default'; state.promise = undefined; - selections.clear(); - setState(({ error }) => ({ error })); } }, diff --git a/packages/subscriptions/package.json b/packages/subscriptions/package.json index bf7c13357..01e407c5e 100644 --- a/packages/subscriptions/package.json +++ b/packages/subscriptions/package.json @@ -31,7 +31,7 @@ "prepare": "bob-esbuild build", "postpublish": "gh-release", "start": "bob-esbuild watch", - "test": "NODE_OPTIONS=--experimental-vm-modules jest" + "test": "jest" }, "dependencies": { "isomorphic-ws": "^5.0.0",