Feat lit-query: add initial lit-query package#10501
Feat lit-query: add initial lit-query package#10501chughgaurav wants to merge 5 commits intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Lit Host
participant Controller as Query Controller
participant Context as Lit Context
participant Client as QueryClient
Host->>Controller: addController(host)
Controller->>Controller: queueMicrotask(onConnected)
Host->>Controller: hostConnected()
Controller->>Context: dispatch ContextEvent(queryClientContext)
Context-->>Controller: resolved QueryClient or undefined
alt client available
Controller->>Client: create Observer & subscribe
Client-->>Controller: emit initial result/updates
Controller->>Host: requestUpdate()
else missing client
Controller->>Controller: set missing-client placeholder
end
Host->>Controller: hostDisconnected()
Controller->>Client: unsubscribe observer (if any)
sequenceDiagram
participant Provider as QueryClientProvider
participant Client as QueryClient
participant Consumer as Consumer Component
participant Context as Lit Context
Provider->>Client: mount() & registerDefaultQueryClient()
Provider->>Context: set context value
Consumer->>Context: dispatch ContextEvent
Context-->>Consumer: return provider's Client
Consumer->>Client: subscribe observer
Client-->>Consumer: emit updates
Consumer->>Consumer: render updates
Provider->>Client: unmount()
Provider->>Context: clear context
Consumer->>Consumer: fallback -> missing-client behavior
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (4)
packages/lit-query/src/useIsFetching.ts (1)
13-108: Extract the shared counter-controller lifecycle.
IsFetchingControlleris now almost identical toIsMutatingControllerinpackages/lit-query/src/useIsMutating.ts; the only real differences are which cache is subscribed to and whichQueryClientcounter is read. Pulling the shared sync/subscribe lifecycle into a small generic helper will keep future fixes from drifting between the two hooks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/src/useIsFetching.ts` around lines 13 - 108, The IsFetchingController and IsMutatingController share nearly identical lifecycle logic (constructor, syncClient, subscribe/unsubscribe, onConnected/onDisconnected/onHostUpdate/onQueryClientChanged); extract that into a small reusable generic helper or base class (e.g., SharedCounterController or lifecycle mixin) that accepts the cache-subscribe function and the counter-read function as parameters, then refactor IsFetchingController and IsMutatingController to delegate to that helper: keep unique methods computeValue (or the counter-read lambda) and the cache selection (queryClient.getQueryCache() vs queryClient.getMutationCache()) as injectable dependencies, reuse tryGetQueryClient/connectedState from BaseController, and ensure subscribe() uses the injected cache subscribe and the result setter from BaseController so both controllers share the sync/subscribe lifecycle.packages/lit-query/examples/lit-query-pagination-app/vite.config.ts (1)
5-14: Optional: deduplicate identicalserver/previewsettings.This is correct as-is; a tiny DRY refactor can reduce future drift.
♻️ Proposed refactor
import { defineConfig } from 'vite' import { DEMO_PORT } from './config/ports.js' +const localServerConfig = () => ({ + host: '127.0.0.1', + port: DEMO_PORT, + strictPort: true, +}) + export default defineConfig({ - server: { - host: '127.0.0.1', - port: DEMO_PORT, - strictPort: true, - }, - preview: { - host: '127.0.0.1', - port: DEMO_PORT, - strictPort: true, - }, + server: localServerConfig(), + preview: localServerConfig(), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/examples/lit-query-pagination-app/vite.config.ts` around lines 5 - 14, Duplicate server and preview configs (server and preview) both use identical settings; extract the shared settings into a single constant (e.g., commonHostConfig or hostConfig) that references DEMO_PORT and then use that constant for both server and preview (via direct assignment or spreading) so changes to host, port or strictPort only need to be made in one place.packages/lit-query/scripts/check-cjs-types-smoke.mjs (1)
46-47: Add defensive check fornpm pack --jsonoutput.If
npm packfails or returns an empty array, the destructuringconst [{ filename }]will throw an unclear error. Consider adding validation.♻️ Proposed defensive parsing
- const [{ filename }] = JSON.parse(stdout) + const packResults = JSON.parse(stdout) + if (!Array.isArray(packResults) || packResults.length === 0 || !packResults[0].filename) { + throw new Error(`Unexpected npm pack output: ${stdout}`) + } + const [{ filename }] = packResults return resolve(destination, filename)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/scripts/check-cjs-types-smoke.mjs` around lines 46 - 47, The current destructuring const [{ filename }] = JSON.parse(stdout) can throw if npm pack returns empty or invalid JSON; change parse logic in this module to validate the parsed value before destructuring: parse stdout with JSON.parse(stdout), ensure the result is an array with at least one element and that result[0].filename is a string, and only then call resolve(destination, filename); otherwise throw or return a clear error indicating npm pack returned no artifacts (include stdout in the error for debugging). Ensure checks are applied where JSON.parse(stdout) and resolve(destination, filename) are used so the code fails with a clear message instead of an uncaught destructuring error.packages/lit-query/src/tests/context-provider.test.ts (1)
16-31: Consider adding afterEach cleanup for DOM elements.The test appends elements to
document.bodyand removes them manually, but if an assertion fails beforeprovider.remove(), the element remains in the DOM and could affect subsequent tests.♻️ Suggested pattern for test isolation
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' // At describe level: let testContainer: HTMLDivElement beforeEach(() => { testContainer = document.createElement('div') document.body.append(testContainer) }) afterEach(() => { testContainer.remove() }) // Then in tests, append to testContainer instead: testContainer.append(provider)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/src/tests/context-provider.test.ts` around lines 16 - 31, Add test-level DOM isolation by creating a test container in beforeEach and cleaning it up in afterEach, then append the QueryClientProvider instance (provider) to that container instead of document.body; update the test suite around the use of QueryClientProvider, useQueryClient, and resolveQueryClient to rely on the testContainer so that provider.remove() is not required for cleanup and the DOM is always removed even if assertions fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/lit-query/examples/lit-query-e2e-app/config/port.js`:
- Around line 9-12: The port validation currently uses Number.parseInt on
envPort which accepts prefixes like "4173abc"; update the logic around
envPort/parsedPort/isValidPort to first verify envPort contains only digits
(e.g., /^\d+$/) before parsing, then parse to a number and enforce
Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535 so
non-numeric suffixes are rejected; adjust any error/default branch to rely on
this stricter check.
In `@packages/lit-query/examples/lit-query-pagination-app/config/ports.js`:
- Around line 10-12: The current logic uses Number.parseInt(rawValue, 10) and
only checks the numeric range, which accepts strings with trailing characters;
update the validation in ports.js so rawValue is first tested against a
full-digit regex (e.g., /^\d+$/) before parsing, then parse with
Number.parseInt(rawValue, 10) into parsed and keep the existing range check
(parsed >= 1 && parsed <= 65535) to set valid; reference the variables rawValue,
parsed and valid when making this change.
In `@packages/lit-query/examples/lit-query-pagination-app/server/index.mjs`:
- Around line 37-60: Both parsePositiveInt and parseNonNegativeInt currently use
Number.parseInt which accepts partial numeric strings like "1abc"; change them
to first verify the entire rawValue is a pure integer string (e.g., use a regex
that matches only digits for non-negative values, and digits for positive
checks) before converting, then parse with Number.parseInt or Number() and apply
the existing bounds checks; ensure the functions still return fallback when
rawValue is null/empty and return undefined for malformed or out-of-range
values.
In `@packages/lit-query/examples/lit-query-pagination-app/src/api.ts`:
- Around line 68-85: readJsonOrThrow currently catches JSON parse errors and
returns null, hiding malformed/empty 2xx bodies; change its logic so you do not
swallow parse errors for successful responses: attempt to parse the body with
response.json() without a blanket .catch for OK responses (let the parse error
propagate or explicitly throw a descriptive error for malformed JSON when
response.ok), while preserving tolerant behavior for non-ok responses (e.g., try
to parse error payload and if parsing fails fall back to the provided
fallbackMessage). Update the implementation in readJsonOrThrow to parse and
handle errors accordingly so successful responses never return null due to
swallowed JSON parse failures.
In `@packages/lit-query/examples/lit-query-ssr-app/scripts/dev.mjs`:
- Around line 23-39: The child process `server` can emit an `error` (spawn
failure) that will never resolve the awaited `once(server, 'exit')`, so modify
the shutdown/wait logic around `server` (and keep `stopServer`) to race the
`'error'` event with `'exit'`/`'close'`: create Promises using `once(server,
'error')` and `once(server, 'exit')` (or `'close'`) and use Promise.race to
await whichever happens first, then on `error` ensure you log/throw the error so
the script fails fast instead of hanging.
In
`@packages/lit-query/examples/lit-query-ssr-app/scripts/run-script-with-public-origin.mjs`:
- Around line 32-38: The code awaits the child process 'exit' event which can
hang if a spawn 'error' occurs; update the wait to listen for 'close' instead of
'exit' (i.e., change the await once(child, 'exit') usage to once(child,
'close')) and keep the existing child.on('error', ...) handler so
process.exitCode is set on errors; this ensures the wrapper always proceeds
after either 'exit' or 'error' and that stdio is closed (look for the
variables/function names child and once and the lines awaiting the 'exit'
event).
In `@packages/lit-query/examples/lit-query-ssr-app/src/api.ts`:
- Around line 51-57: The queryFn in createDataQueryOptions currently ignores the
TanStack Query context and doesn't forward the AbortSignal, breaking
cancellation; update the queryFn signature to accept the context parameter (from
createDataQueryOptions → queryFn: async ({ signal }) => { ... }) and pass that
signal into fetch(resolveApiUrl('/api/data', apiBaseUrl), { signal }); keep
using DATA_QUERY_KEY and return readJson<DataResponse>(response) unchanged so
cancellation is preserved when unmounting or aborting queries.
In `@packages/lit-query/package.json`:
- Around line 85-93: The package manifest currently pins "@tanstack/query-core"
to a published version under "dependencies"; replace that entry with a workspace
link by setting "@tanstack/query-core": "workspace:*" in the package.json
dependencies (keep the peerDependencies constraint as-is) so local monorepo
changes to query-core are used during builds and tests; update the
"dependencies" object where "@tanstack/query-core" appears to the workspace
spec.
In `@packages/lit-query/README.md`:
- Around line 216-233: The README's "Quality Gates" section references missing
docs files (`docs/TEST_MATRIX.md`, `docs/TEST_MATRIX_INTEGRATION.md`,
`docs/TEST_MATRIX_PERF.md`, `docs/RFC-v4.1.md`); either add those document files
with the expected content or remove/replace those links in the README to avoid
broken references. Update the section in packages/lit-query/README.md to either
point to existing documents or to inline the current gate commands (e.g., the
`Current local quality gate` and `Full local demo gate`) so contributors aren't
directed to non-existent files.
In `@packages/lit-query/scripts/check-package.mjs`:
- Around line 8-11: The execFileSync call that assigns rawOutput uses the
literal 'npm' which fails on Windows because the executable is 'npm.cmd'; update
the invocation in check-package.mjs to pick the correct binary based on platform
(e.g., determine npmBinary = process.platform === 'win32' ? 'npm.cmd' : 'npm' or
derive from process.env.npm_execpath) and call execFileSync(npmBinary,
['pack','--dry-run','--json'], { cwd: repoRoot, encoding: 'utf8' }) so the
script works on Windows and POSIX systems; keep the same args and options and
only change the binary selection logic around the execFileSync usage that sets
rawOutput.
In `@packages/lit-query/src/createMutationController.ts`:
- Around line 55-83: The idle fallback mutation result returned in
createMutationController/createIdleMutationResult is missing the public
MutationObserverResult field isPaused, causing mutation().isPaused to be
undefined; add isPaused: false to the returned object literal (alongside
isIdle/isPending/isSuccess/isError) so the shape matches
MutationObserverResult<TData,TError,TVariables,TOnMutateResult> and consumers
receive a boolean in the idle state.
In `@packages/lit-query/src/tests/client-switch-controllers.test.ts`:
- Around line 312-320: Tests currently call custom-element lifecycle methods
directly (consumer.connectedCallback() and consumer.disconnectedCallback()),
which double-runs controller hooks; instead remove those manual calls and use
DOM operations to drive lifecycle: use providerA.removeChild(consumer) and
providerB.append(consumer) (or append consumer to document.body to simulate
“connected without a provider”) so the browser invokes
disconnectedCallback/connectedCallback naturally; update the assertions around
consumer.mutation.mutateAsync to run after the DOM-based attach/detach and apply
the same fix to the other occurrences mentioned (lines near 377-389 and 451-465)
referencing providerA, providerB and consumer.
In `@packages/lit-query/src/tests/counters-and-state.test.ts`:
- Line 1: explicitCountersClient is shared mutable state causing leaks if a test
fails before the manual cleanup at the end of the file; move its teardown into
an afterEach hook so every test resets it. Add an afterEach that checks
explicitCountersClient and, if set, calls its close/dispose/shutdown method (or
nulls it) and then assigns undefined (or resets to initial value); remove the
one-off cleanup at the end of the file so teardown always runs even on test
failures.
In `@packages/lit-query/src/tests/infinite-and-options.test.ts`:
- Line 1: The tests leak shared state by cleaning up explicitInfiniteClient
inline; move the explicitInfiniteClient teardown into an afterEach hook so it's
always disposed even if a test fails (reference the explicitInfiniteClient
variable and the test suite in infinite-and-options.test.ts), i.e., add an
afterEach that checks for and closes/disposes explicitInfiniteClient and resets
it to undefined, and remove the inline cleanup to ensure each test runs with a
fresh provider/client.
In `@packages/lit-query/src/tests/testHost.ts`:
- Around line 86-97: The waitFor helper currently only checks boolean returns
and fails to handle assertion functions that throw or return undefined; update
waitFor to support sync or async assertions by calling await
Promise.resolve(assertion()) inside a try/catch loop, treat any truthy
(non-false) return value as success and return immediately, and on exceptions
capture the thrown error into a lastError variable and continue polling until
timeout, at which point throw the lastError (or a timeout Error if none) so
failing expectations bubble correctly; modify the function named waitFor
accordingly to implement this behavior.
---
Nitpick comments:
In `@packages/lit-query/examples/lit-query-pagination-app/vite.config.ts`:
- Around line 5-14: Duplicate server and preview configs (server and preview)
both use identical settings; extract the shared settings into a single constant
(e.g., commonHostConfig or hostConfig) that references DEMO_PORT and then use
that constant for both server and preview (via direct assignment or spreading)
so changes to host, port or strictPort only need to be made in one place.
In `@packages/lit-query/scripts/check-cjs-types-smoke.mjs`:
- Around line 46-47: The current destructuring const [{ filename }] =
JSON.parse(stdout) can throw if npm pack returns empty or invalid JSON; change
parse logic in this module to validate the parsed value before destructuring:
parse stdout with JSON.parse(stdout), ensure the result is an array with at
least one element and that result[0].filename is a string, and only then call
resolve(destination, filename); otherwise throw or return a clear error
indicating npm pack returned no artifacts (include stdout in the error for
debugging). Ensure checks are applied where JSON.parse(stdout) and
resolve(destination, filename) are used so the code fails with a clear message
instead of an uncaught destructuring error.
In `@packages/lit-query/src/tests/context-provider.test.ts`:
- Around line 16-31: Add test-level DOM isolation by creating a test container
in beforeEach and cleaning it up in afterEach, then append the
QueryClientProvider instance (provider) to that container instead of
document.body; update the test suite around the use of QueryClientProvider,
useQueryClient, and resolveQueryClient to rely on the testContainer so that
provider.remove() is not required for cleanup and the DOM is always removed even
if assertions fail.
In `@packages/lit-query/src/useIsFetching.ts`:
- Around line 13-108: The IsFetchingController and IsMutatingController share
nearly identical lifecycle logic (constructor, syncClient,
subscribe/unsubscribe,
onConnected/onDisconnected/onHostUpdate/onQueryClientChanged); extract that into
a small reusable generic helper or base class (e.g., SharedCounterController or
lifecycle mixin) that accepts the cache-subscribe function and the counter-read
function as parameters, then refactor IsFetchingController and
IsMutatingController to delegate to that helper: keep unique methods
computeValue (or the counter-read lambda) and the cache selection
(queryClient.getQueryCache() vs queryClient.getMutationCache()) as injectable
dependencies, reuse tryGetQueryClient/connectedState from BaseController, and
ensure subscribe() uses the injected cache subscribe and the result setter from
BaseController so both controllers share the sync/subscribe lifecycle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33495bdd-700c-4c8d-b2ea-f30ed64c2f1e
📒 Files selected for processing (95)
packages/lit-query/.editorconfigpackages/lit-query/.npmignorepackages/lit-query/.prettierignorepackages/lit-query/README.mdpackages/lit-query/eslint.config.jspackages/lit-query/examples/lit-query-e2e-app/basic-query.htmlpackages/lit-query/examples/lit-query-e2e-app/config/port.d.tspackages/lit-query/examples/lit-query-e2e-app/config/port.jspackages/lit-query/examples/lit-query-e2e-app/e2e/lib.mjspackages/lit-query/examples/lit-query-e2e-app/e2e/lifecycle-contract.mjspackages/lit-query/examples/lit-query-e2e-app/e2e/lifecycle-reconnect.mjspackages/lit-query/examples/lit-query-e2e-app/e2e/mutation-error.mjspackages/lit-query/examples/lit-query-e2e-app/e2e/query-error.mjspackages/lit-query/examples/lit-query-e2e-app/e2e/refetch-button.mjspackages/lit-query/examples/lit-query-e2e-app/e2e/smoke.mjspackages/lit-query/examples/lit-query-e2e-app/index.htmlpackages/lit-query/examples/lit-query-e2e-app/lifecycle-contract.htmlpackages/lit-query/examples/lit-query-e2e-app/mutation.htmlpackages/lit-query/examples/lit-query-e2e-app/package.jsonpackages/lit-query/examples/lit-query-e2e-app/src/basic-query.tspackages/lit-query/examples/lit-query-e2e-app/src/lifecycle-contract.tspackages/lit-query/examples/lit-query-e2e-app/src/main.tspackages/lit-query/examples/lit-query-e2e-app/src/mutation.tspackages/lit-query/examples/lit-query-e2e-app/src/todoApi.tspackages/lit-query/examples/lit-query-e2e-app/tsconfig.jsonpackages/lit-query/examples/lit-query-e2e-app/vite.config.tspackages/lit-query/examples/lit-query-pagination-app/config/ports.d.tspackages/lit-query/examples/lit-query-pagination-app/config/ports.jspackages/lit-query/examples/lit-query-pagination-app/e2e/boundary.mjspackages/lit-query/examples/lit-query-pagination-app/e2e/error-recovery.mjspackages/lit-query/examples/lit-query-pagination-app/e2e/lib.mjspackages/lit-query/examples/lit-query-pagination-app/e2e/mutations.mjspackages/lit-query/examples/lit-query-pagination-app/e2e/prefetch.mjspackages/lit-query/examples/lit-query-pagination-app/e2e/smoke.mjspackages/lit-query/examples/lit-query-pagination-app/index.htmlpackages/lit-query/examples/lit-query-pagination-app/package.jsonpackages/lit-query/examples/lit-query-pagination-app/scripts/dev.mjspackages/lit-query/examples/lit-query-pagination-app/server/index.mjspackages/lit-query/examples/lit-query-pagination-app/src/api.tspackages/lit-query/examples/lit-query-pagination-app/src/main.tspackages/lit-query/examples/lit-query-pagination-app/src/vite-env.d.tspackages/lit-query/examples/lit-query-pagination-app/tsconfig.jsonpackages/lit-query/examples/lit-query-pagination-app/vite.config.tspackages/lit-query/examples/lit-query-ssr-app/config/ports.d.tspackages/lit-query/examples/lit-query-ssr-app/config/ports.jspackages/lit-query/examples/lit-query-ssr-app/e2e/error-recovery.mjspackages/lit-query/examples/lit-query-ssr-app/e2e/lib.mjspackages/lit-query/examples/lit-query-ssr-app/e2e/refreshing.mjspackages/lit-query/examples/lit-query-ssr-app/e2e/ssr-smoke.mjspackages/lit-query/examples/lit-query-ssr-app/index.htmlpackages/lit-query/examples/lit-query-ssr-app/package.jsonpackages/lit-query/examples/lit-query-ssr-app/scripts/dev.mjspackages/lit-query/examples/lit-query-ssr-app/scripts/run-script-with-public-origin.mjspackages/lit-query/examples/lit-query-ssr-app/server/index.mjspackages/lit-query/examples/lit-query-ssr-app/src/api.tspackages/lit-query/examples/lit-query-ssr-app/src/app.tspackages/lit-query/examples/lit-query-ssr-app/src/main.tspackages/lit-query/examples/lit-query-ssr-app/tsconfig.jsonpackages/lit-query/examples/lit-query-ssr-app/vite.config.tspackages/lit-query/package.jsonpackages/lit-query/prettier.config.jspackages/lit-query/scripts/check-cjs-types-smoke.mjspackages/lit-query/scripts/check-package.mjspackages/lit-query/scripts/l3-stress.mjspackages/lit-query/scripts/measure-bundle.mjspackages/lit-query/scripts/write-cjs-package.mjspackages/lit-query/src/QueryClientProvider.tspackages/lit-query/src/accessor.tspackages/lit-query/src/context.tspackages/lit-query/src/controllers/BaseController.tspackages/lit-query/src/createInfiniteQueryController.tspackages/lit-query/src/createMutationController.tspackages/lit-query/src/createQueriesController.tspackages/lit-query/src/createQueryController.tspackages/lit-query/src/index.tspackages/lit-query/src/infiniteQueryOptions.tspackages/lit-query/src/mutationOptions.tspackages/lit-query/src/queryOptions.tspackages/lit-query/src/tests/client-switch-controllers.test.tspackages/lit-query/src/tests/context-provider.test.tspackages/lit-query/src/tests/counters-and-state.test.tspackages/lit-query/src/tests/infinite-and-options.test.tspackages/lit-query/src/tests/mutation-controller.test.tspackages/lit-query/src/tests/queries-controller.test.tspackages/lit-query/src/tests/query-controller.test.tspackages/lit-query/src/tests/testHost.tspackages/lit-query/src/tests/type-inference.test.tspackages/lit-query/src/types.tspackages/lit-query/src/useIsFetching.tspackages/lit-query/src/useIsMutating.tspackages/lit-query/src/useMutationState.tspackages/lit-query/tsconfig.build.cjs.jsonpackages/lit-query/tsconfig.build.jsonpackages/lit-query/tsconfig.jsonpackages/lit-query/vitest.config.ts
| const parsedPort = Number.parseInt(envPort, 10) | ||
| const isValidPort = | ||
| Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript, does Number.parseInt("4173abc", 10) return 4173, and what is the recommended strict validation pattern for environment variable ports?
💡 Result:
Yes, Number.parseInt("4173abc", 10) returns 4173. This is because parseInt parses until it encounters a non-digit character in the specified radix (base 10 here), ignoring trailing invalid characters. The recommended strict validation pattern for environment variable ports in Node.js combines parsing with validation to ensure the entire string consists only of valid digits (no trailing junk, decimals, etc.) and the value is in the valid port range (typically 0-65535, often >0 and <=65535 for TCP ports). A common pattern, seen in Express generator's normalizePort and various best practices: function normalizePort(val) { const port = parseInt(val, 10); if (isNaN(port)) return val; // Allow named pipes if (port >= 0) return port; return false; } For stricter integer-only validation (rejecting anything not purely digits): - Test if the string matches /^\d+$/ (optionally allowing leading + or - if needed, but ports are unsigned). - Then parseInt and check range. Example strict port parser: function parseStrictPort(str) { if (!/^\d+$/.test(str)) return NaN; const port = Number.parseInt(str, 10); return (port >= 0 && port <= 65535) ? port : NaN; } Usage: const port = parseStrictPort(process.env.PORT); if (isNaN(port)) { throw new Error('Invalid PORT'); }
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/parseInt
- 2: https://udn.realityripple.com/docs/Web/JavaScript/Reference/Global_Objects/parseInt
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt?retiredLocale=de
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt
- 5: https://www.techonthenet.com/js/number_parseint.php
- 6: https://stackoverflow.com/questions/43417975/parse-an-integer-and-only-an-integer-in-javascript
- 7: https://stackoverflow.com/questions/10834796/validate-that-a-string-is-a-positive-integer
- 8: https://stackoverflow.com/questions/37674069/javascript-check-if-string-can-be-converted-to-integer
- 9: https://stackoverflow.com/questions/55988881/what-does-normalizeport-function-do-in-nodejs
🏁 Script executed:
cat -n packages/lit-query/examples/lit-query-e2e-app/config/port.js | sed -n '1,20p'Repository: TanStack/query
Length of output: 649
Use strict numeric validation for DEMO_PORT.
Lines 9-12 currently accept values like "4173abc" because Number.parseInt is prefix-tolerant, silently accepting invalid environment input. Enforce digit-only validation before parsing.
Proposed fix
- const parsedPort = Number.parseInt(envPort, 10)
- const isValidPort =
- Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535
+ const isDigitsOnly = /^\d+$/.test(envPort)
+ const parsedPort = Number(envPort)
+ const isValidPort =
+ isDigitsOnly && Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const parsedPort = Number.parseInt(envPort, 10) | |
| const isValidPort = | |
| Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535 | |
| const isDigitsOnly = /^\d+$/.test(envPort) | |
| const parsedPort = Number(envPort) | |
| const isValidPort = | |
| isDigitsOnly && Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/examples/lit-query-e2e-app/config/port.js` around lines 9
- 12, The port validation currently uses Number.parseInt on envPort which
accepts prefixes like "4173abc"; update the logic around
envPort/parsedPort/isValidPort to first verify envPort contains only digits
(e.g., /^\d+$/) before parsing, then parse to a number and enforce
Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535 so
non-numeric suffixes are rejected; adjust any error/default branch to rely on
this stricter check.
| const parsed = Number.parseInt(rawValue, 10) | ||
| const valid = Number.isInteger(parsed) && parsed >= 1 && parsed <= 65_535 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does JavaScript parseInt accept trailing non-numeric characters, and how should a port env var be validated to ensure the whole string is numeric?
💡 Result:
Yes, JavaScript's parseInt accepts trailing non-numeric characters and parses only the leading numeric portion, returning the integer value up to the first invalid character. For example, parseInt("123abc", 10) returns 123. To validate a port environment variable (e.g., process.env.PORT) and ensure the whole string is numeric (a valid integer port 0-65535), use this approach: 1. Trim whitespace. 2. Parse with parseInt(string, 10). 3. Check if the parsed number equals the original trimmed string (ensures no trailing invalid chars). 4. Verify it's an integer within port range. Example code: function validatePort(str) { const trimmed = str.trim; if (trimmed === '') return false; const num = parseInt(trimmed, 10); return num >= 0 && num <= 65535 && trimmed === num.toString; } // Usage: const portStr = process.env.PORT; const port = parseInt(portStr, 10); if (validatePort(portStr) && port !== NaN) { // Use port } else { // Handle invalid: use default like 3000 } This catches cases like "3000foo" (parses to 3000 but validation fails), "3000.5" (parses to 3000 but string mismatch), empty, non-numeric. Alternatively, use regex /^\d+$/ for digits-only check before parsing, or libraries like envalid.port or zod.coerce.number.int.min(0).max(65535).
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt.
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt
- 3: https://stackoverflow.com/questions/21060645/js-validation-ipport
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isInteger
- 5: https://stackoverflow.com/questions/37674069/javascript-check-if-string-can-be-converted-to-integer
- 6: https://stackoverflow.com/questions/10834796/validate-that-a-string-is-a-positive-integer
🏁 Script executed:
find . -name "ports.js" -type f | head -20Repository: TanStack/query
Length of output: 192
🏁 Script executed:
if [ -f "packages/lit-query/examples/lit-query-pagination-app/config/ports.js" ]; then
echo "=== File exists ==="
wc -l "packages/lit-query/examples/lit-query-pagination-app/config/ports.js"
echo "=== Lines 1-20 ==="
sed -n '1,20p' "packages/lit-query/examples/lit-query-pagination-app/config/ports.js"
fiRepository: TanStack/query
Length of output: 616
Tighten env port parsing to reject malformed strings with trailing characters.
Number.parseInt() accepts strings with trailing non-numeric characters (e.g., "4184foo" parses as 4184). Line 10-12 would incorrectly validate such inputs since it only checks if the parsed result is a valid integer in range, not if the entire input string is numeric.
Use a regex check before parsing to ensure the full string contains only digits:
Proposed fix
- const parsed = Number.parseInt(rawValue, 10)
- const valid = Number.isInteger(parsed) && parsed >= 1 && parsed <= 65_535
+ const isDigitsOnly = /^\d+$/.test(rawValue)
+ const parsed = Number(rawValue)
+ const valid = isDigitsOnly && Number.isInteger(parsed) && parsed >= 1 && parsed <= 65_535🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/examples/lit-query-pagination-app/config/ports.js` around
lines 10 - 12, The current logic uses Number.parseInt(rawValue, 10) and only
checks the numeric range, which accepts strings with trailing characters; update
the validation in ports.js so rawValue is first tested against a full-digit
regex (e.g., /^\d+$/) before parsing, then parse with Number.parseInt(rawValue,
10) into parsed and keep the existing range check (parsed >= 1 && parsed <=
65535) to set valid; reference the variables rawValue, parsed and valid when
making this change.
| function parsePositiveInt(rawValue, fallback) { | ||
| if (rawValue == null || rawValue === '') { | ||
| return fallback | ||
| } | ||
|
|
||
| const parsed = Number.parseInt(rawValue, 10) | ||
| if (!Number.isInteger(parsed) || parsed < 1) { | ||
| return undefined | ||
| } | ||
|
|
||
| return parsed | ||
| } | ||
|
|
||
| function parseNonNegativeInt(rawValue, fallback) { | ||
| if (rawValue == null || rawValue === '') { | ||
| return fallback | ||
| } | ||
|
|
||
| const parsed = Number.parseInt(rawValue, 10) | ||
| if (!Number.isInteger(parsed) || parsed < 0) { | ||
| return undefined | ||
| } | ||
|
|
||
| return parsed |
There was a problem hiding this comment.
Reject partial numeric strings here.
Number.parseInt will accept inputs like "1abc" and "10ms", so these helpers currently treat malformed query params as valid. If the endpoint is meant to validate integers, use a full-string check before converting.
Safer integer parsing
function parsePositiveInt(rawValue, fallback) {
if (rawValue == null || rawValue === '') {
return fallback
}
- const parsed = Number.parseInt(rawValue, 10)
- if (!Number.isInteger(parsed) || parsed < 1) {
+ if (!/^[1-9]\d*$/.test(rawValue)) {
return undefined
}
+ const parsed = Number(rawValue)
return parsed
}
function parseNonNegativeInt(rawValue, fallback) {
if (rawValue == null || rawValue === '') {
return fallback
}
- const parsed = Number.parseInt(rawValue, 10)
- if (!Number.isInteger(parsed) || parsed < 0) {
+ if (!/^\d+$/.test(rawValue)) {
return undefined
}
+ const parsed = Number(rawValue)
return parsed
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/examples/lit-query-pagination-app/server/index.mjs` around
lines 37 - 60, Both parsePositiveInt and parseNonNegativeInt currently use
Number.parseInt which accepts partial numeric strings like "1abc"; change them
to first verify the entire rawValue is a pure integer string (e.g., use a regex
that matches only digits for non-negative values, and digits for positive
checks) before converting, then parse with Number.parseInt or Number() and apply
the existing bounds checks; ensure the functions still return fallback when
rawValue is null/empty and return undefined for malformed or out-of-range
values.
| async function readJsonOrThrow<T>( | ||
| response: Response, | ||
| fallbackMessage: string, | ||
| ): Promise<T> { | ||
| const payload = (await response.json().catch(() => null)) as | ||
| | { error?: string } | ||
| | T | ||
| | null | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error( | ||
| payload && typeof payload === 'object' && 'error' in payload | ||
| ? String(payload.error ?? fallbackMessage) | ||
| : fallbackMessage, | ||
| ) | ||
| } | ||
|
|
||
| return payload as T |
There was a problem hiding this comment.
Don't swallow JSON parse failures on successful responses.
response.json().catch(() => null) turns a malformed or empty 2xx body into null as T, so the real protocol error gets hidden and the app fails later with a much less useful error.
💡 Suggested fix
async function readJsonOrThrow<T>(
response: Response,
fallbackMessage: string,
): Promise<T> {
- const payload = (await response.json().catch(() => null)) as
+ const payload = (await response.json().catch(() => {
+ if (response.ok) {
+ throw new Error(`${fallbackMessage}: invalid JSON response`)
+ }
+ return null
+ })) as
| { error?: string }
| T
| null
if (!response.ok) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function readJsonOrThrow<T>( | |
| response: Response, | |
| fallbackMessage: string, | |
| ): Promise<T> { | |
| const payload = (await response.json().catch(() => null)) as | |
| | { error?: string } | |
| | T | |
| | null | |
| if (!response.ok) { | |
| throw new Error( | |
| payload && typeof payload === 'object' && 'error' in payload | |
| ? String(payload.error ?? fallbackMessage) | |
| : fallbackMessage, | |
| ) | |
| } | |
| return payload as T | |
| async function readJsonOrThrow<T>( | |
| response: Response, | |
| fallbackMessage: string, | |
| ): Promise<T> { | |
| const payload = (await response.json().catch(() => { | |
| if (response.ok) { | |
| throw new Error(`${fallbackMessage}: invalid JSON response`) | |
| } | |
| return null | |
| })) as | |
| | { error?: string } | |
| | T | |
| | null | |
| if (!response.ok) { | |
| throw new Error( | |
| payload && typeof payload === 'object' && 'error' in payload | |
| ? String(payload.error ?? fallbackMessage) | |
| : fallbackMessage, | |
| ) | |
| } | |
| return payload as T |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/examples/lit-query-pagination-app/src/api.ts` around lines
68 - 85, readJsonOrThrow currently catches JSON parse errors and returns null,
hiding malformed/empty 2xx bodies; change its logic so you do not swallow parse
errors for successful responses: attempt to parse the body with response.json()
without a blanket .catch for OK responses (let the parse error propagate or
explicitly throw a descriptive error for malformed JSON when response.ok), while
preserving tolerant behavior for non-ok responses (e.g., try to parse error
payload and if parsing fails fall back to the provided fallbackMessage). Update
the implementation in readJsonOrThrow to parse and handle errors accordingly so
successful responses never return null due to swallowed JSON parse failures.
| const server = spawn(npmCommand, ['run', 'dev:server'], { | ||
| cwd, | ||
| stdio: 'inherit', | ||
| }) | ||
|
|
||
| const stopServer = (signal) => { | ||
| if (server.exitCode === null) { | ||
| server.kill(signal) | ||
| } | ||
| } | ||
|
|
||
| process.on('SIGINT', () => stopServer('SIGINT')) | ||
| process.on('SIGTERM', () => stopServer('SIGTERM')) | ||
|
|
||
| const [code] = await once(server, 'exit') | ||
| process.exitCode = code ?? 0 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "dev.mjs" | head -20Repository: TanStack/query
Length of output: 192
🌐 Web query:
Node.js child_process spawn error event vs exit event semantics
💡 Result:
In Node.js child_process.spawn, the 'error' event and 'exit' event have distinct semantics: The 'error' event is emitted for failures in process management, specifically: - The process could not be spawned (e.g., command not found, ENOENT error). - The process could not be killed. - Sending a message to the child process failed. - The child process was aborted via the 'signal' option. If spawning fails, 'error' fires, 'spawn' does not emit, and 'exit' may or may not fire afterward. The docs explicitly warn: "The 'exit' event may or may not fire after an error has occurred. When listening to both the 'exit' and 'error' events, guard against accidentally invoking handler functions multiple times." The 'exit' event is emitted after the child process ends normally or via signal, providing the exit code (if exited) or signal name. It fires regardless of successful spawn (unless spawn error prevents process creation). At 'exit' time, stdio streams might still be open; use 'close' event for when stdio is fully closed. Example distinction: spawn('nonexistent', []) → 'error' (spawn failure), no 'exit'. spawn('rm', ['-f', 'dir']) → process spawns, runs, exits with code 1 → 'exit'(1), no 'error' (runtime failure, not spawn failure). This behavior is consistent across Node.js versions (v18+, v25 docs), as spawn errors are pre-process-creation while exit reports post-spawn termination.
Citations:
- 1: https://nodejs.org/api/child_process.html
- 2: https://nodejs.org/docs/latest-v18.x/api/child_process.html
- 3: https://nodejs.org/dist/latest-v19.x/docs/api/child_process.html
🏁 Script executed:
cat -n ./packages/lit-query/examples/lit-query-ssr-app/scripts/dev.mjsRepository: TanStack/query
Length of output: 1367
Handle child-process error events to prevent hangs on spawn failures.
At line 37, awaiting only once(server, 'exit') can miss spawn failures (error event). Per Node.js semantics, if spawning fails (e.g., command not found), the error event fires but exit may never fire, causing the script to hang indefinitely. The outer .catch() does not catch unhandled error events emitted on the ChildProcess object. Race error with close (or exit) so startup failures always fail fast.
Suggested fix
- const [code] = await once(server, 'exit')
- process.exitCode = code ?? 0
+ const [code] = await Promise.race([
+ once(server, 'close'),
+ once(server, 'error').then(([error]) => {
+ throw error
+ }),
+ ])
+ process.exitCode = code ?? 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const server = spawn(npmCommand, ['run', 'dev:server'], { | |
| cwd, | |
| stdio: 'inherit', | |
| }) | |
| const stopServer = (signal) => { | |
| if (server.exitCode === null) { | |
| server.kill(signal) | |
| } | |
| } | |
| process.on('SIGINT', () => stopServer('SIGINT')) | |
| process.on('SIGTERM', () => stopServer('SIGTERM')) | |
| const [code] = await once(server, 'exit') | |
| process.exitCode = code ?? 0 | |
| } | |
| const server = spawn(npmCommand, ['run', 'dev:server'], { | |
| cwd, | |
| stdio: 'inherit', | |
| }) | |
| const stopServer = (signal) => { | |
| if (server.exitCode === null) { | |
| server.kill(signal) | |
| } | |
| } | |
| process.on('SIGINT', () => stopServer('SIGINT')) | |
| process.on('SIGTERM', () => stopServer('SIGTERM')) | |
| const [code] = await Promise.race([ | |
| once(server, 'close'), | |
| once(server, 'error').then(([error]) => { | |
| throw error | |
| }), | |
| ]) | |
| process.exitCode = code ?? 0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/examples/lit-query-ssr-app/scripts/dev.mjs` around lines
23 - 39, The child process `server` can emit an `error` (spawn failure) that
will never resolve the awaited `once(server, 'exit')`, so modify the
shutdown/wait logic around `server` (and keep `stopServer`) to race the
`'error'` event with `'exit'`/`'close'`: create Promises using `once(server,
'error')` and `once(server, 'exit')` (or `'close'`) and use Promise.race to
await whichever happens first, then on `error` ensure you log/throw the error so
the script fails fast instead of hanging.
| return { | ||
| context: undefined, | ||
| data: undefined, | ||
| error: null, | ||
| failureCount: 0, | ||
| failureReason: null, | ||
| isError: false, | ||
| isIdle: true, | ||
| isPending: false, | ||
| isSuccess: false, | ||
| status: 'idle', | ||
| submittedAt: 0, | ||
| variables: undefined, | ||
| mutate: (() => | ||
| Promise.reject( | ||
| createMissingQueryClientError(), | ||
| )) as MutationObserverResult< | ||
| TData, | ||
| TError, | ||
| TVariables, | ||
| TOnMutateResult | ||
| >['mutate'], | ||
| reset: (() => undefined) as MutationObserverResult< | ||
| TData, | ||
| TError, | ||
| TVariables, | ||
| TOnMutateResult | ||
| >['reset'], | ||
| } as MutationObserverResult<TData, TError, TVariables, TOnMutateResult> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "createMutationController.ts" -type fRepository: TanStack/query
Length of output: 1854
🏁 Script executed:
git ls-files packages/lit-query/src/ | head -20Repository: TanStack/query
Length of output: 1854
🌐 Web query:
TanStack Query MutationObserverResult isPaused type definition
💡 Result:
The MutationObserverResult type in TanStack Query (from @tanstack/query-core) includes an isPaused property defined as a boolean. This indicates whether the mutation is currently paused, typically due to network issues (offline mode), mutation scopes, or other pausing mechanisms. Complete type definition: interface MutationObserverResult<TData, TError, TVariables, TContext> { context: TContext | undefined data: TData | undefined error: TError | null failureCount: number failureReason: TError | null isError: boolean isIdle: boolean isPending: boolean isPaused: boolean isSuccess: boolean mutate: (variables: TVariables, options?: MutateOptions) => void mutateAsync: (variables: TVariables, options?: MutateOptions) => Promise<TData> reset: => void status: 'idle' | 'pending' | 'error' | 'success' submittedAt: number variables: TVariables | undefined } This is the return type of MutationObserver.getCurrentResult and what useMutation returns in React integrations. The isPaused boolean is derived in the source code from the mutation state (e.g., when status is 'pending' but paused due to offline/network conditions or scopes). Official docs confirm isPaused is available on mutation results for network mode and scope features.
Citations:
- 1: https://github.com/TanStack/query/blob/main/packages/query-core/src/mutationObserver.ts
- 2: https://mintlify.com/TanStack/query/api/mutation-observer
- 3: https://www.mintlify.com/TanStack/query/api/mutation-observer
🌐 Web query:
@tanstack/query-core MutationState isPaused field
💡 Result:
Yes, the MutationState interface in @tanstack/query-core includes an isPaused field of type boolean. The MutationState interface is defined as: interface MutationState<TData, TError, TVariables, TContext> { context: TContext | undefined data: TData | undefined error: TError | null failureCount: number failureReason: TError | null isPaused: boolean status: 'idle' | 'pending' | 'error' | 'success' variables: TVariables | undefined submittedAt: number } This field indicates whether the mutation is currently paused, typically due to network conditions (offline), mutation scopes, or persister restoration scenarios. When true, the mutation is queued and will resume when conditions allow (e.g., via queryClient.resumePausedMutations). The isPaused state is exposed in hooks like useMutation (as isPaused boolean) and useMutationState, and used internally by MutationCache to filter paused mutations (e.g., this.#mutations.filter((x) => x.state.isPaused)). Recent fixes (PR #9015, Apr 2025) ensure restored mutations correctly set isPaused to false upon continuation.
Citations:
- 1: https://www.mintlify.com/TanStack/query/api/mutation
- 2:
isPausedstaystruewhen resuming mutations from the cache #8999 - 3: Solve inconsistencies between "paused" mutations and "paused" queries #6760
- 4: fix(query-core): unpause restored mutations #9015
- 5: https://tanstack.com/query/latest/docs/framework/react/reference/useMutation
- 6: https://github.com/TanStack/query/blob/main/packages/query-core/src/mutationObserver.ts
Add isPaused to the idle fallback mutation result.
The public MutationObserverResult interface from TanStack Query includes isPaused: boolean, which is exposed to consumers via mutation hooks. The current createIdleMutationResult() fallback omits this field, so mutation().isPaused returns undefined instead of the expected boolean value during the idle state.
Proposed fix
return {
context: undefined,
data: undefined,
error: null,
failureCount: 0,
failureReason: null,
+ isPaused: false,
isError: false,
isIdle: true,
isPending: false,
isSuccess: false,
status: 'idle',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| context: undefined, | |
| data: undefined, | |
| error: null, | |
| failureCount: 0, | |
| failureReason: null, | |
| isError: false, | |
| isIdle: true, | |
| isPending: false, | |
| isSuccess: false, | |
| status: 'idle', | |
| submittedAt: 0, | |
| variables: undefined, | |
| mutate: (() => | |
| Promise.reject( | |
| createMissingQueryClientError(), | |
| )) as MutationObserverResult< | |
| TData, | |
| TError, | |
| TVariables, | |
| TOnMutateResult | |
| >['mutate'], | |
| reset: (() => undefined) as MutationObserverResult< | |
| TData, | |
| TError, | |
| TVariables, | |
| TOnMutateResult | |
| >['reset'], | |
| } as MutationObserverResult<TData, TError, TVariables, TOnMutateResult> | |
| return { | |
| context: undefined, | |
| data: undefined, | |
| error: null, | |
| failureCount: 0, | |
| failureReason: null, | |
| isPaused: false, | |
| isError: false, | |
| isIdle: true, | |
| isPending: false, | |
| isSuccess: false, | |
| status: 'idle', | |
| submittedAt: 0, | |
| variables: undefined, | |
| mutate: (() => | |
| Promise.reject( | |
| createMissingQueryClientError(), | |
| )) as MutationObserverResult< | |
| TData, | |
| TError, | |
| TVariables, | |
| TOnMutateResult | |
| >['mutate'], | |
| reset: (() => undefined) as MutationObserverResult< | |
| TData, | |
| TError, | |
| TVariables, | |
| TOnMutateResult | |
| >['reset'], | |
| } as MutationObserverResult<TData, TError, TVariables, TOnMutateResult> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/createMutationController.ts` around lines 55 - 83, The
idle fallback mutation result returned in
createMutationController/createIdleMutationResult is missing the public
MutationObserverResult field isPaused, causing mutation().isPaused to be
undefined; add isPaused: false to the returned object literal (alongside
isIdle/isPending/isSuccess/isError) so the shape matches
MutationObserverResult<TData,TError,TVariables,TOnMutateResult> and consumers
receive a boolean in the idle state.
| providerA.removeChild(consumer) | ||
| await new Promise((resolve) => setTimeout(resolve, 0)) | ||
| consumer.connectedCallback() | ||
| await expect(consumer.mutation.mutateAsync(2)).rejects.toThrow( | ||
| /No QueryClient available/, | ||
| ) | ||
|
|
||
| consumer.disconnectedCallback() | ||
| providerB.append(consumer) |
There was a problem hiding this comment.
Avoid calling custom-element lifecycle callbacks manually.
These tests are putting the host into states the browser never creates. removeChild/append already drive disconnectedCallback and connectedCallback; the extra manual calls double-run controller hooks and can mask real client-resolution or observer-cleanup bugs. To test “connected without a provider”, append the consumer to document.body instead of invoking the callbacks directly.
Also applies to: 377-389, 451-465
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/tests/client-switch-controllers.test.ts` around lines
312 - 320, Tests currently call custom-element lifecycle methods directly
(consumer.connectedCallback() and consumer.disconnectedCallback()), which
double-runs controller hooks; instead remove those manual calls and use DOM
operations to drive lifecycle: use providerA.removeChild(consumer) and
providerB.append(consumer) (or append consumer to document.body to simulate
“connected without a provider”) so the browser invokes
disconnectedCallback/connectedCallback naturally; update the assertions around
consumer.mutation.mutateAsync to run after the DOM-based attach/detach and apply
the same fix to the other occurrences mentioned (lines near 377-389 and 451-465)
referencing providerA, providerB and consumer.
| @@ -0,0 +1,604 @@ | |||
| import { describe, expect, it } from 'vitest' | |||
There was a problem hiding this comment.
Move explicitCountersClient cleanup into afterEach.
explicitCountersClient is shared mutable state. If an assertion fails before Line 211 runs, later tests in this file stop covering the provider path and start using the leaked explicit client instead.
🧪 Suggested test hardening
-import { describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it } from 'vitest'
import { QueryClient } from '@tanstack/query-core'
@@
let explicitCountersClient: QueryClient | undefined
+
+afterEach(() => {
+ explicitCountersClient = undefined
+})Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/tests/counters-and-state.test.ts` at line 1,
explicitCountersClient is shared mutable state causing leaks if a test fails
before the manual cleanup at the end of the file; move its teardown into an
afterEach hook so every test resets it. Add an afterEach that checks
explicitCountersClient and, if set, calls its close/dispose/shutdown method (or
nulls it) and then assigns undefined (or resets to initial value); remove the
one-off cleanup at the end of the file so teardown always runs even on test
failures.
| @@ -0,0 +1,377 @@ | |||
| import { describe, expect, it } from 'vitest' | |||
There was a problem hiding this comment.
Move explicitInfiniteClient cleanup into afterEach.
This file has the same shared-state leak risk: if LC-INF-02 fails before Line 129, later tests can accidentally bypass provider coverage by reusing the leaked explicit client.
🧪 Suggested test hardening
-import { describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it } from 'vitest'
import { QueryClient } from '@tanstack/query-core'
@@
let explicitInfiniteClient: QueryClient | undefined
+
+afterEach(() => {
+ explicitInfiniteClient = undefined
+})Also applies to: 23-23
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/tests/infinite-and-options.test.ts` at line 1, The
tests leak shared state by cleaning up explicitInfiniteClient inline; move the
explicitInfiniteClient teardown into an afterEach hook so it's always disposed
even if a test fails (reference the explicitInfiniteClient variable and the test
suite in infinite-and-options.test.ts), i.e., add an afterEach that checks for
and closes/disposes explicitInfiniteClient and resets it to undefined, and
remove the inline cleanup to ensure each test runs with a fresh provider/client.
| export async function waitFor( | ||
| assertion: () => boolean, | ||
| timeoutMs = 2000, | ||
| ): Promise<void> { | ||
| const startedAt = Date.now() | ||
| while (!assertion()) { | ||
| if (Date.now() - startedAt > timeoutMs) { | ||
| throw new Error(`Timed out waiting for assertion after ${timeoutMs}ms`) | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| } | ||
| } |
There was a problem hiding this comment.
waitFor only handles boolean predicates right now.
A passing callback like () => expect(x).toBe(1) returns undefined, so this loop keeps polling until timeout. Thrown assertions also abort immediately instead of retrying, which makes the helper brittle for normal test usage.
💡 Suggested fix
export async function waitFor(
- assertion: () => boolean,
+ assertion: () => unknown,
timeoutMs = 2000,
): Promise<void> {
const startedAt = Date.now()
- while (!assertion()) {
- if (Date.now() - startedAt > timeoutMs) {
- throw new Error(`Timed out waiting for assertion after ${timeoutMs}ms`)
+ let lastError: unknown
+
+ while (Date.now() - startedAt <= timeoutMs) {
+ try {
+ const result = assertion()
+ if (result !== false) {
+ return
+ }
+ } catch (error) {
+ lastError = error
}
+
await new Promise((resolve) => setTimeout(resolve, 10))
}
+
+ throw lastError ?? new Error(`Timed out waiting for assertion after ${timeoutMs}ms`)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function waitFor( | |
| assertion: () => boolean, | |
| timeoutMs = 2000, | |
| ): Promise<void> { | |
| const startedAt = Date.now() | |
| while (!assertion()) { | |
| if (Date.now() - startedAt > timeoutMs) { | |
| throw new Error(`Timed out waiting for assertion after ${timeoutMs}ms`) | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, 10)) | |
| } | |
| } | |
| export async function waitFor( | |
| assertion: () => unknown, | |
| timeoutMs = 2000, | |
| ): Promise<void> { | |
| const startedAt = Date.now() | |
| let lastError: unknown | |
| while (Date.now() - startedAt <= timeoutMs) { | |
| try { | |
| const result = assertion() | |
| if (result !== false) { | |
| return | |
| } | |
| } catch (error) { | |
| lastError = error | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, 10)) | |
| } | |
| throw lastError ?? new Error(`Timed out waiting for assertion after ${timeoutMs}ms`) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/tests/testHost.ts` around lines 86 - 97, The waitFor
helper currently only checks boolean returns and fails to handle assertion
functions that throw or return undefined; update waitFor to support sync or
async assertions by calling await Promise.resolve(assertion()) inside a
try/catch loop, treat any truthy (non-false) return value as success and return
immediately, and on exceptions capture the thrown error into a lastError
variable and continue polling until timeout, at which point throw the lastError
(or a timeout Error if none) so failing expectations bubble correctly; modify
the function named waitFor accordingly to implement this behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/lit-query/src/useMutationState.ts (2)
113-118: Simplify condition:selectis always a function by type definition.Since
selectis typed as(mutation: Mutation) => TResult, thetypeofcheck is redundant—it will always be'function'whenselectis defined. Using!== undefinedis clearer.♻️ Suggested simplification
private shouldRefreshOnHostUpdate(): boolean { return ( typeof this.options.filters === 'function' || - typeof this.options.select === 'function' + this.options.select !== undefined ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/src/useMutationState.ts` around lines 113 - 118, The condition in shouldRefreshOnHostUpdate() uses typeof this.options.select === 'function' but select is statically typed as a function when present; change the check to test presence instead (e.g., this.options.select !== undefined) and keep the filters check as-is (typeof this.options.filters === 'function'); update the shouldRefreshOnHostUpdate method to use the presence check for options.select to simplify the logic and avoid a redundant typeof check.
54-58: Consider removing unnecessarysyncClient()call inonDisconnected.After unsubscribing on line 55-56, calling
syncClient()appears unnecessary—we're disconnecting so the client reference state doesn't need updating, and the return value is discarded.♻️ Suggested simplification
protected onDisconnected(): void { this.unsubscribe?.() this.unsubscribe = undefined - this.syncClient() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/src/useMutationState.ts` around lines 54 - 58, The onDisconnected method calls this.unsubscribe and then calls this.syncClient(), but since we're disconnecting the client the syncClient() call is unnecessary and its return value is ignored; remove the this.syncClient() call from the onDisconnected() method (leave the unsubscribe clearing logic intact) so only the unsubscribe cleanup runs in onDisconnected.packages/lit-query/src/createQueriesController.ts (2)
179-179: Consider usingundefinedinstead of empty array for initial result.The empty array
[]cast toTCombinedResultcould cause unexpected behavior ifTCombinedResultis not array-like (e.g., whencombinereturns an object). While this value is typically replaced before access, usingundefinedas the initial sentinel and handling it inreadCurrentwould be more type-safe.♻️ Suggested approach
- super(host, [] as unknown as TCombinedResult, queryClient) + super(host, undefined as unknown as TCombinedResult, queryClient)Then ensure
readCurrenthandles the undefined case, which it already does viaplaceholderInitializedtracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/src/createQueriesController.ts` at line 179, The constructor currently passes an empty array cast to TCombinedResult into super (super(host, [] as unknown as TCombinedResult, queryClient)), which is unsafe if TCombinedResult isn't array-like; change that initial result to undefined (pass undefined as the initial sentinel) and ensure the class handles this by using the existing placeholderInitialized logic and readCurrent to return a proper value when the result is undefined; update the super call to use undefined and verify readCurrent and any consumers of the initial value (e.g., placeholderInitialized, readCurrent) correctly handle undefined.
346-361: Document the early-read handshake pattern.The error swallowing based on
placeholderRetryableFailureis clever for handling reads during class-field initialization (before accessors are ready), but this subtle timing behavior could confuse maintainers. Consider adding a brief inline comment explaining the microtask window.📝 Suggested documentation
readCurrent(): TCombinedResult { if (!this.queryClient && !this.observer && !this.placeholderInitialized) { try { - // Early reads can happen during class-field initialization, before - // accessors referenced by queries/combine are ready. Retry normally - // after construction finishes and only surface errors after that point. + // Early reads can happen during class-field initialization, before + // accessors referenced by queries/combine are ready. The queueMicrotask + // in the constructor gives a brief window where errors are swallowed, + // allowing the host to complete initialization before we surface errors. this.setPlaceholderResult() } catch (error) { if (!this.placeholderRetryableFailure) { throw error } + // Error swallowed during handshake window - will retry on next access } } return this.current }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/src/createQueriesController.ts` around lines 346 - 361, Add an inline comment in readCurrent explaining the early-read handshake: document that setPlaceholderResult is retried to handle reads during class-field initialization (a microtask window before accessors are wired), why placeholderRetryableFailure suppresses errors during that window, and when errors will be surfaced (after placeholderInitialized becomes true); reference the readCurrent method, setPlaceholderResult call, placeholderInitialized and placeholderRetryableFailure flags, and the returned current result so future maintainers understand the timing semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/lit-query/src/createQueriesController.ts`:
- Line 179: The constructor currently passes an empty array cast to
TCombinedResult into super (super(host, [] as unknown as TCombinedResult,
queryClient)), which is unsafe if TCombinedResult isn't array-like; change that
initial result to undefined (pass undefined as the initial sentinel) and ensure
the class handles this by using the existing placeholderInitialized logic and
readCurrent to return a proper value when the result is undefined; update the
super call to use undefined and verify readCurrent and any consumers of the
initial value (e.g., placeholderInitialized, readCurrent) correctly handle
undefined.
- Around line 346-361: Add an inline comment in readCurrent explaining the
early-read handshake: document that setPlaceholderResult is retried to handle
reads during class-field initialization (a microtask window before accessors are
wired), why placeholderRetryableFailure suppresses errors during that window,
and when errors will be surfaced (after placeholderInitialized becomes true);
reference the readCurrent method, setPlaceholderResult call,
placeholderInitialized and placeholderRetryableFailure flags, and the returned
current result so future maintainers understand the timing semantics.
In `@packages/lit-query/src/useMutationState.ts`:
- Around line 113-118: The condition in shouldRefreshOnHostUpdate() uses typeof
this.options.select === 'function' but select is statically typed as a function
when present; change the check to test presence instead (e.g.,
this.options.select !== undefined) and keep the filters check as-is (typeof
this.options.filters === 'function'); update the shouldRefreshOnHostUpdate
method to use the presence check for options.select to simplify the logic and
avoid a redundant typeof check.
- Around line 54-58: The onDisconnected method calls this.unsubscribe and then
calls this.syncClient(), but since we're disconnecting the client the
syncClient() call is unnecessary and its return value is ignored; remove the
this.syncClient() call from the onDisconnected() method (leave the unsubscribe
clearing logic intact) so only the unsubscribe cleanup runs in onDisconnected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e23f0f9a-ca96-457f-8103-84e2e95b80a0
📒 Files selected for processing (7)
packages/lit-query/src/createQueriesController.tspackages/lit-query/src/index.tspackages/lit-query/src/queryOptions.tspackages/lit-query/src/tests/counters-and-state.test.tspackages/lit-query/src/tests/type-inference.test.tspackages/lit-query/src/types.tspackages/lit-query/src/useMutationState.ts
✅ Files skipped from review due to trivial changes (1)
- packages/lit-query/src/tests/counters-and-state.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/lit-query/src/types.ts
- packages/lit-query/src/tests/type-inference.test.ts
- packages/lit-query/src/index.ts
- Fix createQueriesController tuple type inference with recursive types - Add DataTag support to queryOptions - Fix build config and vitest custom-condition resolution - Fix example install scripts for standalone bootstrap
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/lit-query/package.json (1)
51-51: Avoid rebuilding twice indemo:gateto reduce CI/dev runtime.
demo:gaterunsbuild, thenpackage:build(which runsbuildagain). Foldingpackage:checkintopackage:buildlets the gate invoke one packaging command without redundant work.♻️ Suggested script cleanup
- "package:build": "pnpm run build && pnpm run test:build", + "package:build": "pnpm run build && pnpm run package:check && pnpm run test:build", ... - "demo:gate": "pnpm run typecheck && pnpm run lint && pnpm run format:check && pnpm test && pnpm run build && pnpm run package:check && pnpm run package:build && pnpm run example:e2e:all:nobuild && pnpm run example:ssr:e2e:all:nobuild && pnpm run measure:bundle:raw && pnpm run perf:l3:raw", + "demo:gate": "pnpm run typecheck && pnpm run lint && pnpm run format:check && pnpm test && pnpm run package:build && pnpm run example:e2e:all:nobuild && pnpm run example:ssr:e2e:all:nobuild && pnpm run measure:bundle:raw && pnpm run perf:l3:raw",Also applies to: 89-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lit-query/package.json` at line 51, Update the npm scripts to remove the duplicate build: change the "package:build" script so it performs the packaging/verification steps currently in "package:check" (fold package:check's duties into package:build) and no longer redundantly runs "build" if the gate already calls build; then adjust "demo:gate" to invoke only "pnpm run package:build" (instead of running "build" and then "package:build") so the CI/dev flow runs the package command once. Ensure references to "package:check", "package:build", and "demo:gate" are updated accordingly and remove or repurpose the old "package:check" script if no longer needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/lit-query/src/createQueriesController.ts`:
- Around line 1-22: The import block mixes type-only and value imports causing
lint errors; split type-only imports from value imports and remove duplicates
for both `@tanstack/query-core` and ./accessor.js: import runtime values
(QueriesObserver, createValueAccessor, readAccessor, BaseController,
createMissingQueryClientError) in one group and import only types (DefaultError,
DefinedQueryObserverResult, OmitKeyof, QueriesObserverOptions, QueryFunction,
QueryKey, QueryObserverOptions, QueryObserverResult, ThrowOnError, QueryClient,
ReactiveControllerHost, Accessor, ValueAccessor) in a separate "import type"
group, and reorder groups to satisfy import/order so all external deps come
before local modules.
- Around line 289-295: The synchronous call to resolveQueriesOptions(options,
queryClient) can throw during construction when an explicit queryClient is
provided; wrap that call and the subsequent QueriesObserver creation and result
computation (resolveQueriesOptions, new QueriesObserver(...),
observer.getCurrentResult(), computeResult) in a try-catch so any thrown error
is caught and the element is left in a safe state (mirroring readCurrent’s
guarded path): on catch assign this.queryClient and this.observer only if
created, set this.result to an error or empty fallback result, and rethrow or
store the error state as appropriate for later handling so construction does not
allow an uncaught exception to propagate.
- Around line 435-442: The placeholders are created with undefined data, so pass
each query's initialData into createPendingQueryObserverResult before calling
resolvedOptions.combine: for each item in queries (obtained via
readAccessor(resolvedOptions.queries)), extract its options.initialData (or
resolved initialData via readAccessor if options is an accessor) and call
createPendingQueryObserverResult(initialData) to build placeholders that satisfy
the DefinedQueryObserverResult contract; then pass that placeholders array to
resolvedOptions.combine (or return placeholders) so consumers (e.g.,
result[0].data.name) see the initial data. Ensure you reference resolvedOptions,
queries, createPendingQueryObserverResult, combine, and readAccessor when making
the change.
In `@packages/lit-query/src/QueryClientProvider.ts`:
- Around line 1-10: The import ordering violates the lint rule because a
type-only import (QueryClient from '@tanstack/query-core') appears before value
imports from 'lit' and './context.js'; reorder the import statements in
QueryClientProvider.ts so that value imports (ContextProvider from
'@lit/context', LitElement, html, TemplateResult from 'lit', and the named
exports createMissingQueryClientError, queryClientContext,
registerDefaultQueryClient, unregisterDefaultQueryClient from './context.js')
come first, and then place the type-only import of QueryClient after them to
satisfy import/order.
- Around line 47-53: When this.client becomes undefined the code currently
throws createMissingQueryClientError() before cleaning up, which leaves the
previously mounted client/state registered; update the QueryClientProvider
branch that checks const nextClient = this.client and if (!nextClient) to first
perform the provider's teardown/cleanup logic for the previous client
(unregister it as the default client and clear subscriptions/state) and then
throw createMissingQueryClientError(); place the cleanup call(s) immediately
before the throw so symbols like client, isConnected and
createMissingQueryClientError remain as-is and only the order changes.
In `@packages/lit-query/src/tests/type-inference.test.ts`:
- Around line 1-7: The import line mixes value and type specifiers causing lint
failures; split into two imports: one importing runtime values (dataTagSymbol,
QueryClient) and a separate type-only import for DefinedQueryObserverResult and
QueryObserverResult using the type keyword (e.g. import { type
DefinedQueryObserverResult, type QueryObserverResult } from
'@tanstack/query-core'), and ensure the two import statements are ordered/sorted
according to project rules (values import first, then type-only import).
---
Nitpick comments:
In `@packages/lit-query/package.json`:
- Line 51: Update the npm scripts to remove the duplicate build: change the
"package:build" script so it performs the packaging/verification steps currently
in "package:check" (fold package:check's duties into package:build) and no
longer redundantly runs "build" if the gate already calls build; then adjust
"demo:gate" to invoke only "pnpm run package:build" (instead of running "build"
and then "package:build") so the CI/dev flow runs the package command once.
Ensure references to "package:check", "package:build", and "demo:gate" are
updated accordingly and remove or repurpose the old "package:check" script if no
longer needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1f340c5-0f5b-4dda-9bc0-d68f3409b53c
📒 Files selected for processing (11)
packages/lit-query/package.jsonpackages/lit-query/scripts/check-cjs-types-smoke.mjspackages/lit-query/src/QueryClientProvider.tspackages/lit-query/src/createQueriesController.tspackages/lit-query/src/queryOptions.tspackages/lit-query/src/tests/type-inference.test.tspackages/lit-query/src/types.tspackages/lit-query/tsconfig.build.cjs.jsonpackages/lit-query/tsconfig.build.jsonpackages/lit-query/tsconfig.jsonpackages/lit-query/vitest.config.ts
✅ Files skipped from review due to trivial changes (3)
- packages/lit-query/tsconfig.build.json
- packages/lit-query/tsconfig.build.cjs.json
- packages/lit-query/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/lit-query/vitest.config.ts
- packages/lit-query/scripts/check-cjs-types-smoke.mjs
- packages/lit-query/src/queryOptions.ts
| import { | ||
| QueriesObserver, | ||
| type DefaultError, | ||
| type DefinedQueryObserverResult, | ||
| type OmitKeyof, | ||
| type QueriesObserverOptions, | ||
| type QueryFunction, | ||
| type QueryKey, | ||
| type QueryObserverOptions, | ||
| type QueryObserverResult, | ||
| type ThrowOnError, | ||
| } from '@tanstack/query-core' | ||
| import type { QueryClient } from '@tanstack/query-core' | ||
| import type { ReactiveControllerHost } from 'lit' | ||
| import { | ||
| createValueAccessor, | ||
| readAccessor, | ||
| type Accessor, | ||
| type ValueAccessor, | ||
| } from './accessor.js' | ||
| import { createMissingQueryClientError } from './context.js' | ||
| import { BaseController } from './controllers/BaseController.js' |
There was a problem hiding this comment.
Normalize this import block before lint runs.
This file currently trips import/consistent-type-specifier-style, import/no-duplicates, and import/order. Split the value imports from the type-only imports for both @tanstack/query-core and ./accessor.js.
Suggested diff
-import {
- QueriesObserver,
- type DefaultError,
- type DefinedQueryObserverResult,
- type OmitKeyof,
- type QueriesObserverOptions,
- type QueryFunction,
- type QueryKey,
- type QueryObserverOptions,
- type QueryObserverResult,
- type ThrowOnError,
-} from '@tanstack/query-core'
-import type { QueryClient } from '@tanstack/query-core'
-import type { ReactiveControllerHost } from 'lit'
-import {
- createValueAccessor,
- readAccessor,
- type Accessor,
- type ValueAccessor,
-} from './accessor.js'
-import { createMissingQueryClientError } from './context.js'
-import { BaseController } from './controllers/BaseController.js'
+import { QueriesObserver } from '@tanstack/query-core'
+import { createValueAccessor, readAccessor } from './accessor.js'
+import { createMissingQueryClientError } from './context.js'
+import { BaseController } from './controllers/BaseController.js'
+
+import type {
+ DefaultError,
+ DefinedQueryObserverResult,
+ OmitKeyof,
+ QueriesObserverOptions,
+ QueryClient,
+ QueryFunction,
+ QueryKey,
+ QueryObserverOptions,
+ QueryObserverResult,
+ ThrowOnError,
+} from '@tanstack/query-core'
+import type { ReactiveControllerHost } from 'lit'
+import type { Accessor, ValueAccessor } from './accessor.js'🧰 Tools
🪛 ESLint
[error] 3-3: Member 'DefaultError' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 3-3: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 4-4: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 5-5: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 6-6: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 7-7: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 8-8: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 9-9: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 10-10: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 11-11: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 12-12: '@tanstack/query-core' imported multiple times.
(import/no-duplicates)
[error] 13-13: @tanstack/query-core type import should occur after import of ./controllers/BaseController.js
(import/order)
[error] 13-13: '@tanstack/query-core' imported multiple times.
(import/no-duplicates)
[error] 14-14: lit type import should occur after import of ./controllers/BaseController.js
(import/order)
[error] 18-18: Member 'Accessor' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 18-18: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 19-19: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/createQueriesController.ts` around lines 1 - 22, The
import block mixes type-only and value imports causing lint errors; split
type-only imports from value imports and remove duplicates for both
`@tanstack/query-core` and ./accessor.js: import runtime values (QueriesObserver,
createValueAccessor, readAccessor, BaseController,
createMissingQueryClientError) in one group and import only types (DefaultError,
DefinedQueryObserverResult, OmitKeyof, QueriesObserverOptions, QueryFunction,
QueryKey, QueryObserverOptions, QueryObserverResult, ThrowOnError, QueryClient,
ReactiveControllerHost, Accessor, ValueAccessor) in a separate "import type"
group, and reorder groups to satisfy import/order so all external deps come
before local modules.
| const { queries, combine } = resolveQueriesOptions(options, queryClient) | ||
| const observer = new QueriesObserver(queryClient, queries, { | ||
| combine, | ||
| } as QueriesObserverOptions<TCombinedResult>) | ||
| this.queryClient = queryClient | ||
| this.observer = observer | ||
| this.result = this.computeResult(observer.getCurrentResult(), combine) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file
fd -t f "createQueriesController.ts" packages/lit-query/src/Repository: TanStack/query
Length of output: 107
🏁 Script executed:
# Read the file to check its size first
wc -l packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 111
🏁 Script executed:
# Read lines 280-300 to see the constructor code
sed -n '280,300p' packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 638
🏁 Script executed:
# Read lines 440-465 to see the readCurrent() method
sed -n '440,465p' packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 870
🏁 Script executed:
# Search for resolveQueriesOptions definition
rg "resolveQueriesOptions" packages/lit-query/src/ -A 10Repository: TanStack/query
Length of output: 2804
🏁 Script executed:
# Get more context on the constructor to see the full flow
sed -n '260,310p' packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 1494
🏁 Script executed:
# Check if there's a try-catch around the constructor code or any protection
sed -n '250,320p' packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 1887
🏁 Script executed:
# Look at the entire setClient method to understand the full flow
rg "private setClient|private.*client" packages/lit-query/src/createQueriesController.ts -A 20Repository: TanStack/query
Length of output: 579
Wrap resolveQueriesOptions() in a try-catch when initializing with an explicit client.
The constructor calls resolveQueriesOptions() synchronously when a queryClient is passed (lines 289-295). If the queries accessor throws during this call, the error will propagate uncaught during element construction. The readCurrent() method handles this hazard for deferred initialization with a try-catch guard, but the explicit-client path bypasses that protection entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/createQueriesController.ts` around lines 289 - 295,
The synchronous call to resolveQueriesOptions(options, queryClient) can throw
during construction when an explicit queryClient is provided; wrap that call and
the subsequent QueriesObserver creation and result computation
(resolveQueriesOptions, new QueriesObserver(...), observer.getCurrentResult(),
computeResult) in a try-catch so any thrown error is caught and the element is
left in a safe state (mirroring readCurrent’s guarded path): on catch assign
this.queryClient and this.observer only if created, set this.result to an error
or empty fallback result, and rethrow or store the error state as appropriate
for later handling so construction does not allow an uncaught exception to
propagate.
| const resolvedOptions = readAccessor(optionsAccessor) | ||
| const queries = readAccessor(resolvedOptions.queries) | ||
| const placeholders = queries.map(() => createPendingQueryObserverResult()) | ||
| return ( | ||
| resolvedOptions.combine | ||
| ? resolvedOptions.combine(placeholders as never) | ||
| : placeholders | ||
| ) as TCombinedResult |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file in question
find . -name "createQueriesController.ts" -type fRepository: TanStack/query
Length of output: 109
🏁 Script executed:
# Read the file and examine lines 435-442
sed -n '430,450p' ./packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 944
🏁 Script executed:
# Find where createPendingQueryObserverResult is defined
rg "createPendingQueryObserverResult" --type ts --type tsx -B 2 -A 5Repository: TanStack/query
Length of output: 85
🏁 Script executed:
# Find where createPendingQueryObserverResult is defined
rg "createPendingQueryObserverResult" -B 2 -A 5Repository: TanStack/query
Length of output: 1369
🏁 Script executed:
# Get more context on the createPendingQueryObserverResult function
rg "function createPendingQueryObserverResult" -A 15Repository: TanStack/query
Length of output: 1264
🏁 Script executed:
# Get the complete createPendingQueryObserverResult function
rg "function createPendingQueryObserverResult" -A 25Repository: TanStack/query
Length of output: 1998
🏁 Script executed:
# Get the complete createPendingQueryObserverResult function - let's get more lines
sed -n '/^function createPendingQueryObserverResult/,/^}/p' ./packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 915
🏁 Script executed:
# Now let's check the referenced test file
sed -n '95,125p' ./packages/lit-query/src/tests/type-inference.test.tsRepository: TanStack/query
Length of output: 993
🏁 Script executed:
# Let's look for DefinedQueryObserverResult in the codebase
rg "DefinedQueryObserverResult" -B 2 -A 5Repository: TanStack/query
Length of output: 16111
🏁 Script executed:
# Let's look at the query options structure to understand if initialData is available
rg "initialData" packages/lit-query/src/createQueriesController.ts -B 2 -A 2Repository: TanStack/query
Length of output: 163
🏁 Script executed:
# Let's examine the createPlaceholderResult method more closely and the surrounding context
sed -n '420,460p' ./packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 1421
🏁 Script executed:
# Let's check the type definition of queries in the options
rg "type CreateQueriesControllerOptions" -A 15 packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 554
🏁 Script executed:
# Let's check the CreateQueriesOptions type
rg "type CreateQueriesOptions" -A 10 packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 435
🏁 Script executed:
# Let's see GetCreateQueriesInput and CreateQueriesInputForController
rg "type GetCreateQueriesInput|type CreateQueriesInputForController" -A 5 packages/lit-query/src/createQueriesController.tsRepository: TanStack/query
Length of output: 483
🏁 Script executed:
# Let's look for CreateQueriesInput to understand the full type
rg "type CreateQueriesInput" packages/query-core/src/types.ts -A 20Repository: TanStack/query
Length of output: 40
🏁 Script executed:
# Let's search for CreateQueriesInput across the entire codebase
rg "type CreateQueriesInput" -A 15Repository: TanStack/query
Length of output: 1860
Pass each query's initialData to placeholder creation before invoking combine.
The placeholder results are created with hardcoded data: undefined and immediately passed to combine. This breaks the DefinedQueryObserverResult contract for queries with initialData and causes runtime failures in callbacks like the test case at lines 101-120 of packages/lit-query/src/tests/type-inference.test.ts that expect result[0].data.name to work. The createPendingQueryObserverResult() function should materialize each query's initial data from its options before the placeholders are passed to combine.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/createQueriesController.ts` around lines 435 - 442,
The placeholders are created with undefined data, so pass each query's
initialData into createPendingQueryObserverResult before calling
resolvedOptions.combine: for each item in queries (obtained via
readAccessor(resolvedOptions.queries)), extract its options.initialData (or
resolved initialData via readAccessor if options is an accessor) and call
createPendingQueryObserverResult(initialData) to build placeholders that satisfy
the DefinedQueryObserverResult contract; then pass that placeholders array to
resolvedOptions.combine (or return placeholders) so consumers (e.g.,
result[0].data.name) see the initial data. Ensure you reference resolvedOptions,
queries, createPendingQueryObserverResult, combine, and readAccessor when making
the change.
| import { ContextProvider } from '@lit/context' | ||
| import type { QueryClient } from '@tanstack/query-core' | ||
| import type { TemplateResult } from 'lit' | ||
| import { LitElement, html } from 'lit' | ||
| import { | ||
| createMissingQueryClientError, | ||
| queryClientContext, | ||
| registerDefaultQueryClient, | ||
| unregisterDefaultQueryClient, | ||
| } from './context.js' |
There was a problem hiding this comment.
Fix the import grouping so this file passes lint.
lit and ./context.js need to be ordered before the type-only @tanstack/query-core import, otherwise import/order keeps failing here.
💡 Minimal reorder
import { ContextProvider } from '@lit/context'
-import type { QueryClient } from '@tanstack/query-core'
-import type { TemplateResult } from 'lit'
-import { LitElement, html } from 'lit'
+import { LitElement, html, type TemplateResult } from 'lit'
import {
createMissingQueryClientError,
queryClientContext,
registerDefaultQueryClient,
unregisterDefaultQueryClient,
} from './context.js'
+import type { QueryClient } from '@tanstack/query-core'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ContextProvider } from '@lit/context' | |
| import type { QueryClient } from '@tanstack/query-core' | |
| import type { TemplateResult } from 'lit' | |
| import { LitElement, html } from 'lit' | |
| import { | |
| createMissingQueryClientError, | |
| queryClientContext, | |
| registerDefaultQueryClient, | |
| unregisterDefaultQueryClient, | |
| } from './context.js' | |
| import { ContextProvider } from '@lit/context' | |
| import { LitElement, html, type TemplateResult } from 'lit' | |
| import { | |
| createMissingQueryClientError, | |
| queryClientContext, | |
| registerDefaultQueryClient, | |
| unregisterDefaultQueryClient, | |
| } from './context.js' | |
| import type { QueryClient } from '@tanstack/query-core' |
🧰 Tools
🪛 ESLint
[error] 4-4: lit import should occur before type import of @tanstack/query-core
(import/order)
[error] 5-10: ./context.js import should occur before type import of @tanstack/query-core
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/QueryClientProvider.ts` around lines 1 - 10, The
import ordering violates the lint rule because a type-only import (QueryClient
from '@tanstack/query-core') appears before value imports from 'lit' and
'./context.js'; reorder the import statements in QueryClientProvider.ts so that
value imports (ContextProvider from '@lit/context', LitElement, html,
TemplateResult from 'lit', and the named exports createMissingQueryClientError,
queryClientContext, registerDefaultQueryClient, unregisterDefaultQueryClient
from './context.js') come first, and then place the type-only import of
QueryClient after them to satisfy import/order.
| const nextClient = this.client | ||
| if (!nextClient) { | ||
| if (this.isConnected) { | ||
| throw createMissingQueryClientError() | ||
| } | ||
|
|
||
| return |
There was a problem hiding this comment.
Tear down the previous client before throwing on client = undefined.
Line 50 throws before any cleanup. If a connected provider had already mounted A and client is later cleared, A stays mounted and registered as the default client until disconnect, so the provider keeps stale subscriptions/state alive after the error.
🧹 Minimal cleanup before raising
const nextClient = this.client
if (!nextClient) {
if (this.isConnected) {
+ this.unmountClient(this.mountedClient)
throw createMissingQueryClientError()
}
return
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/QueryClientProvider.ts` around lines 47 - 53, When
this.client becomes undefined the code currently throws
createMissingQueryClientError() before cleaning up, which leaves the previously
mounted client/state registered; update the QueryClientProvider branch that
checks const nextClient = this.client and if (!nextClient) to first perform the
provider's teardown/cleanup logic for the previous client (unregister it as the
default client and clear subscriptions/state) and then throw
createMissingQueryClientError(); place the cleanup call(s) immediately before
the throw so symbols like client, isConnected and createMissingQueryClientError
remain as-is and only the order changes.
| import { | ||
| dataTagSymbol, | ||
| QueryClient, | ||
| type DefinedQueryObserverResult, | ||
| type QueryObserverResult, | ||
| } from '@tanstack/query-core' | ||
| import { describe, expectTypeOf, it } from 'vitest' |
There was a problem hiding this comment.
Split the value and type imports here.
This block currently fails import/consistent-type-specifier-style and sort-imports, so the new type test will not pass lint as written.
Suggested diff
-import {
- dataTagSymbol,
- QueryClient,
- type DefinedQueryObserverResult,
- type QueryObserverResult,
-} from '@tanstack/query-core'
+import { QueryClient, dataTagSymbol } from '@tanstack/query-core'
+import type {
+ DefinedQueryObserverResult,
+ QueryObserverResult,
+} from '@tanstack/query-core'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { | |
| dataTagSymbol, | |
| QueryClient, | |
| type DefinedQueryObserverResult, | |
| type QueryObserverResult, | |
| } from '@tanstack/query-core' | |
| import { describe, expectTypeOf, it } from 'vitest' | |
| import { QueryClient, dataTagSymbol } from '@tanstack/query-core' | |
| import type { | |
| DefinedQueryObserverResult, | |
| QueryObserverResult, | |
| } from '@tanstack/query-core' | |
| import { describe, expectTypeOf, it } from 'vitest' |
🧰 Tools
🪛 ESLint
[error] 3-3: Member 'QueryClient' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 4-4: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
[error] 5-5: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lit-query/src/tests/type-inference.test.ts` around lines 1 - 7, The
import line mixes value and type specifiers causing lint failures; split into
two imports: one importing runtime values (dataTagSymbol, QueryClient) and a
separate type-only import for DefinedQueryObserverResult and QueryObserverResult
using the type keyword (e.g. import { type DefinedQueryObserverResult, type
QueryObserverResult } from '@tanstack/query-core'), and ensure the two import
statements are ordered/sorted according to project rules (values import first,
then type-only import).
1 participant
🎯 Changes
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests / E2E