🤖🤖🤖 fix: disable MockedProvider devtools by default - #13348
Conversation
✅ AI Style Review — No Changes DetectedNo MDX files were changed in this pull request. Review Log: View detailed log
|
|
@cyphercodes: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
🦋 Changeset detectedLatest commit: 015f8fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthrough
ChangesMockedProvider DevTools behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/testing/react/__tests__/MockedProvider.test.tsx`:
- Around line 104-115: Update the “should allow Apollo Client DevTools to be
enabled” test to flush the pending DevTools fallback timer and assert timer
cleanup while window is still active, after unmounting the MockedProvider. Wrap
fake-timer setup and test execution in a finally block that always restores real
timers, preserving the existing DevTools and global-client assertions.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5680d913-3124-4695-924e-a9b49362ab89
📒 Files selected for processing (3)
.changeset/proud-cooks-sip.mdsrc/testing/react/MockedProvider.tsxsrc/testing/react/__tests__/MockedProvider.test.tsx
| it("should allow Apollo Client DevTools to be enabled", () => { | ||
| jest.useFakeTimers(); | ||
| delete (window as any).__APOLLO_CLIENT__; | ||
|
|
||
| const provider = new MockedProvider({ devtools: { enabled: true } }); | ||
|
|
||
| expect((provider as any).state.client.devtoolsConfig.enabled).toBe(true); | ||
| expect((window as any).__APOLLO_CLIENT__).toBeDefined(); | ||
|
|
||
| delete (window as any).__APOLLO_CLIENT__; | ||
| provider.componentWillUnmount(); | ||
| jest.useRealTimers(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -a 'MockedProvider.test.tsx|MockedProvider.tsx|.*devtools.*' . | sed 's#^\./##' | head -100
echo "== target file outline/size =="
if [ -f src/testing/react/__tests__/MockedProvider.test.tsx ]; then
wc -l src/testing/react/__tests__/MockedProvider.test.tsx
ast-grep outline src/testing/react/__tests__/MockedProvider.test.tsx || true
sed -n '1,180p' src/testing/react/__tests__/MockedProvider.test.tsx
fi
echo "== related devtools/MockedProvider sources =="
rg -n "devtoolsConfig|__APOLLO_CLIENT__|devtools:|setTimeout|setInterval|teardown|useFakeTimers|useRealTimers" src testing . -g '!node_modules' -g '!dist' -g '!build' | head -200Repository: apollographql/apollo-client
Length of output: 25398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/testing/react/__tests__/MockedProvider.test.tsx')
if not p.exists():
print("target_missing")
raise SystemExit
s = p.read_text()
for needle in ['delete (window as any).__APOLLO_CLIENT__', 'MockedProvider({ devtools: { enabled: true } })', '.flushAllTimers', 'runOnlyPendingTimers', 'runOnlyPendingTimersAsync', 'flushAllTimersAsync', 'useRealTimers', "afterEach"]:
print(f"{needle!r}: {needle in s}")
# Print relevant block with line numbers.
lines = s.splitlines()
for i,l in enumerate(lines,1):
if 100 <= i <= 125:
print(f"{i}: {l}")
PYRepository: apollographql/apollo-client
Length of output: 1394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ApolloClient devtools code =="
sed -n '1080,1215p' src/core/ApolloClient.ts
echo "== MockedProvider code =="
sed -n '1,105p' src/testing/react/MockedProvider.tsx
echo "== enableFakeTimers helper =="
sed -n '1,80p' src/testing/internal/disposables/enableFakeTimers.ts
echo "== setup fake timers globally if present =="
sed -n '1,140p' src/config/jest/setup.tsRepository: apollographql/apollo-client
Length of output: 10089
Flush the DevTools fallback timer while window is still active.
connectToDevTools() schedules the 10s fallback timeout before MockedProvider can call client.stop(), but this test never advances or asserts pending timers before jsdom teardown. Run pending timers and assert they clean up while window is still available, and restore real timers in a finally block so assertion failures cannot leave other tests running on fake timers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/testing/react/__tests__/MockedProvider.test.tsx` around lines 104 - 115,
Update the “should allow Apollo Client DevTools to be enabled” test to flush the
pending DevTools fallback timer and assert timer cleanup while window is still
active, after unmounting the MockedProvider. Wrap fake-timer setup and test
execution in a finally block that always restores real timers, preserving the
existing DevTools and global-client assertions.
| }), | ||
| localState, | ||
| devtools, | ||
| devtools: devtools ?? { enabled: false }, |
There was a problem hiding this comment.
That doesn't solve the actual problem.
Any other ApolloClient instance created in similar situations will still cause the same result.
|
Your PR doesn't actually solve the underlying problem but merely a symptom, and we're going a different direction. I'm sorry, but please don't open drive-by AI-PRs without discussing the solution you're going for on the issue first. We're going with #13349 which is a solution that @jerelmiller and I had already discussed yesterday. If you had checked in with us first, we could have saved you the money for the tokens 😞 |
3 participants
Fixes #13344
Summary
MockedProviderbehavior from MockedProvider: defaultconnectToDevTools: false#11289 by disabling Apollo Client DevTools unlessdevtoolsis explicitly provided.window.__APOLLO_CLIENT__whiledevtools={{ enabled: true }}still opts in.Checklist:
Tests
npm run test -- --runInBand --testRegex src/testing/react/__tests__/MockedProvider.test.tsxnpm exec -- prettier --check src/testing/react/MockedProvider.tsx src/testing/react/__tests__/MockedProvider.test.tsx .changeset/proud-cooks-sip.mdgit diff --checkNote: a focused ESLint run was attempted, but the local clone lacked generated
docs/public/canonical-references.json, causing ESLint config loading to fail before linting changed files.Summary by CodeRabbit
Bug Fixes
MockedProvider, preventing unintended global DevTools registration.Documentation