Skip to content

Commit

Permalink
switch useRenderGuard to an approach not accessing React's internals (
Browse files Browse the repository at this point in the history
  • Loading branch information
phryneas authored Jun 12, 2024
1 parent 6536369 commit 7fb7939
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 61 deletions.
5 changes: 5 additions & 0 deletions .changeset/brown-bikes-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

switch `useRenderGuard` to an approach not accessing React's internals
2 changes: 1 addition & 1 deletion .size-limits.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 39620,
"dist/apollo-client.min.cjs": 39561,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32821
}
43 changes: 0 additions & 43 deletions src/react/hooks/internal/__tests__/useRenderGuard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import React, { useEffect } from "rehackt";
import { useRenderGuard } from "../useRenderGuard";
import { render, waitFor } from "@testing-library/react";
import { withCleanup } from "../../../../testing/internal";

const UNDEF = {};
const IS_REACT_19 = React.version.startsWith("19");
Expand Down Expand Up @@ -35,45 +34,3 @@ it("returns a function that returns `false` if called after render", async () =>
});
expect(result).toBe(false);
});

function breakReactInternalsTemporarily() {
const R = React as unknown as {
__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: any;
};
const orig = R.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;

R.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED = {};
return withCleanup({}, () => {
R.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED = orig;
});
}

it("results in false negatives if React internals change", () => {
let result: boolean | typeof UNDEF = UNDEF;
function TestComponent() {
using _ = breakReactInternalsTemporarily();
const calledDuringRender = useRenderGuard();
result = calledDuringRender();
return <>Test</>;
}
render(<TestComponent />);
expect(result).toBe(false);
});

it("does not result in false positives if React internals change", async () => {
let result: boolean | typeof UNDEF = UNDEF;
function TestComponent() {
using _ = breakReactInternalsTemporarily();
const calledDuringRender = useRenderGuard();
useEffect(() => {
using _ = breakReactInternalsTemporarily();
result = calledDuringRender();
});
return <>Test</>;
}
render(<TestComponent />);
await waitFor(() => {
expect(result).not.toBe(UNDEF);
});
expect(result).toBe(false);
});
56 changes: 39 additions & 17 deletions src/react/hooks/internal/useRenderGuard.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,45 @@
import * as React from "rehackt";

function getRenderDispatcher() {
return (React as any).__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
?.ReactCurrentDispatcher?.current;
}

let RenderDispatcher: unknown = null;
let Ctx: React.Context<null>;

/*
Relay does this too, so we hope this is safe.
https://github.com/facebook/relay/blob/8651fbca19adbfbb79af7a3bc40834d105fd7747/packages/react-relay/relay-hooks/loadQuery.js#L90-L98
*/
function noop() {}
export function useRenderGuard() {
// eslint-disable-next-line react-compiler/react-compiler
RenderDispatcher = getRenderDispatcher();
if (!Ctx) {
// we want the intialization to be lazy because `createContext` would error on import in a RSC
Ctx = React.createContext(null);
}

return React.useCallback(
/**
* @returns true if the hook was called during render
*/ () => {
const orig = console.error;
try {
console.error = noop;

return React.useCallback(() => {
return (
RenderDispatcher != null && RenderDispatcher === getRenderDispatcher()
);
}, []);
/**
* `useContext` can be called conditionally during render, so this is safe.
* (Also, during render we would want to throw as a reaction to this anyways, so it
* wouldn't even matter if we got the order of hooks mixed up...)
*
* They cannot however be called outside of Render, and that's what we're testing here.
*
* Different versions of React have different behaviour on an invalid hook call:
*
* React 16.8 - 17: throws an error
* https://github.com/facebook/react/blob/2b93d686e359c7afa299e2ec5cf63160a32a1155/packages/react/src/ReactHooks.js#L18-L26
*
* React 18 & 19: `console.error` in development, then `resolveDispatcher` returns `null` and a member access on `null` throws.
* https://github.com/facebook/react/blob/58e8304483ebfadd02a295339b5e9a989ac98c6e/packages/react/src/ReactHooks.js#L28-L35
*/
React["useContext" /* hide this from the linter */](Ctx);
return true;
} catch (e) {
return false;
} finally {
console.error = orig;
}
},
[]
);
}

0 comments on commit 7fb7939

Please sign in to comment.