Skip to content

Commit 49c5c1e

Browse files
authored
fix: improve effect provenance diagnostics (#1452)
1 parent 4fbab2d commit 49c5c1e

49 files changed

Lines changed: 5485 additions & 381 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"oxlint-plugin-react-doctor": patch
3+
---
4+
5+
Improve diagnostic precision for effect cleanup, derived state, parent callbacks, deferred state transitions, and KaTeX HTML rendering.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// rule: dangerous-html-sink
2+
// weakness: control-flow
3+
// source: React Bench Datastoria audit review
4+
5+
import katex from "katex";
6+
7+
interface MathPreviewProps {
8+
options: object;
9+
value: string;
10+
}
11+
12+
export const MathPreview = ({ options, value }: MathPreviewProps) => (
13+
<>
14+
<span
15+
dangerouslySetInnerHTML={{
16+
// eslint-disable-next-line no-constant-binary-expression
17+
__html: false && katex.renderToString(value, options),
18+
}}
19+
/>
20+
<span
21+
dangerouslySetInnerHTML={{
22+
// eslint-disable-next-line no-constant-condition
23+
__html: (true ? "ready" : katex.renderToString(value, options)) ?? "",
24+
}}
25+
/>
26+
</>
27+
);
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// rule: dangerous-html-sink
2+
// weakness: default-parameter
3+
// source: React Bench Datastoria audit review
4+
5+
import katex from "katex";
6+
7+
const renderMath = (value: string, options: object = { throwOnError: false }): string =>
8+
katex.renderToString(value, options);
9+
10+
const renderWithDependentDefault = (
11+
value: string,
12+
baseOptions: object,
13+
options: object = baseOptions,
14+
): string => katex.renderToString(value, options);
15+
16+
const renderWithDestructuredDefault = (
17+
value: string,
18+
{ options = { trust: false } }: { options?: object } = {},
19+
): string => katex.renderToString(value, options);
20+
21+
export const MathPreview = ({ value }: { value: string }) => (
22+
<>
23+
<span dangerouslySetInnerHTML={{ __html: renderMath(value) }} />
24+
<span
25+
dangerouslySetInnerHTML={{
26+
__html: renderWithDependentDefault(value, { trust: false }),
27+
}}
28+
/>
29+
<span dangerouslySetInnerHTML={{ __html: renderWithDestructuredDefault(value, {}) }} />
30+
</>
31+
);
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// rule: effect-needs-cleanup
2+
// verdict: pass
3+
// weakness: library-idiom
4+
// source: react-bench write-react-azouaoui-med-react-pro-sidebar-267 6Z552od false positive
5+
6+
import React from "react";
7+
8+
const getServerSnapshot = () => false;
9+
const noopUnsubscribe = () => undefined;
10+
11+
export const useMediaQuery = (breakpoint?: string): boolean => {
12+
const mediaQueryList = React.useMemo(
13+
() =>
14+
breakpoint && typeof window !== "undefined" && typeof window.matchMedia === "function"
15+
? window.matchMedia(breakpoint)
16+
: null,
17+
[breakpoint],
18+
);
19+
20+
const subscribe = React.useCallback(
21+
(onStoreChange: () => void) => {
22+
if (!mediaQueryList) return noopUnsubscribe;
23+
if (typeof mediaQueryList.addEventListener === "function") {
24+
mediaQueryList.addEventListener("change", onStoreChange);
25+
return () => mediaQueryList.removeEventListener("change", onStoreChange);
26+
}
27+
mediaQueryList.addListener(onStoreChange);
28+
return () => mediaQueryList.removeListener(onStoreChange);
29+
},
30+
[mediaQueryList],
31+
);
32+
33+
const getSnapshot = React.useCallback(() => Boolean(mediaQueryList?.matches), [mediaQueryList]);
34+
35+
return React.useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
36+
};
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// rule: no-adjust-state-on-prop-change
2+
// verdict: pass
3+
// weakness: library-idiom
4+
// source: verified React Bench SlideshowContext trial 5sz2qXB
5+
6+
import * as React from "react";
7+
import { useTimeouts } from "./timeouts";
8+
9+
interface SlideshowProps {
10+
disabled: boolean;
11+
}
12+
13+
export const Slideshow = ({ disabled }: SlideshowProps) => {
14+
const [playing, setPlaying] = React.useState(true);
15+
const { setTimeout, clearTimeout } = useTimeouts();
16+
const scheduler = React.useRef<ReturnType<typeof setTimeout>>();
17+
18+
const cancelScheduler = React.useCallback(() => {
19+
clearTimeout(scheduler.current);
20+
scheduler.current = undefined;
21+
}, [clearTimeout]);
22+
23+
React.useEffect(() => {
24+
if (playing && disabled) {
25+
cancelScheduler();
26+
setPlaying(false);
27+
}
28+
}, [playing, disabled, cancelScheduler]);
29+
30+
return playing;
31+
};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// rule: no-prop-callback-in-effect
2+
// weakness: alias-guard
3+
// source: parent callback provenance review
4+
// verdict: pass
5+
6+
import { useEffect, useRef, useState } from "react";
7+
8+
declare const overwriteRef: (reference: { current: (value: number) => void }) => void;
9+
10+
export const Child = ({ onChange }: { onChange: (value: number) => void }) => {
11+
const [value] = useState(0);
12+
const callbackRef = useRef(onChange);
13+
overwriteRef(callbackRef);
14+
useEffect(() => callbackRef.current(value), [value]);
15+
return null;
16+
};
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// rule: dangerous-html-sink
2+
// weakness: option-provenance
3+
// source: React Bench Datastoria audit
4+
5+
import katex from "katex";
6+
7+
interface MathPreviewProps {
8+
options: object;
9+
value: string;
10+
}
11+
12+
export const MathPreview = ({ options, value }: MathPreviewProps) => (
13+
<span dangerouslySetInnerHTML={{ __html: katex.renderToString(value, options) }} />
14+
);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// rule: effect-needs-cleanup
2+
// verdict: fail
3+
// weakness: identity-provenance
4+
// source: adversarial review of memoized MediaQueryList cleanup ownership
5+
6+
import React from "react";
7+
8+
export const CustomMemoizedListener = ({ customBus, shouldUseMediaQuery }) => {
9+
const memoizedBus = React.useMemo(() => {
10+
let bus = customBus;
11+
if (shouldUseMediaQuery) {
12+
bus = window.matchMedia("(prefers-color-scheme: dark)");
13+
}
14+
return bus;
15+
}, [customBus, shouldUseMediaQuery]);
16+
17+
const subscribe = React.useCallback(
18+
(handle) => {
19+
memoizedBus.addListener(handle);
20+
return () => memoizedBus.removeListener(handle);
21+
},
22+
[memoizedBus],
23+
);
24+
25+
return React.useSyncExternalStore(subscribe, getSnapshot);
26+
};
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// rule: no-adjust-state-on-prop-change
2+
// weakness: alias-guard
3+
// source: AppFlowy DocumentHistoryModal React Bench uonQqwc
4+
5+
import { useEffect, useMemo, useState } from "react";
6+
7+
interface Version {
8+
versionId: string;
9+
visible: boolean;
10+
}
11+
12+
export const DocumentHistoryModal = ({ versions }: { versions: Version[] }) => {
13+
const visibleVersions = useMemo(() => versions.filter((version) => version.visible), [versions]);
14+
const [selectedVersionId, setSelectedVersionId] = useState("");
15+
16+
useEffect(() => {
17+
if (visibleVersions.some((version) => version.versionId === selectedVersionId)) return;
18+
setSelectedVersionId(visibleVersions[0].versionId);
19+
}, [visibleVersions]);
20+
21+
return (
22+
<VersionList
23+
versions={visibleVersions}
24+
selectedVersionId={selectedVersionId}
25+
onSelect={setSelectedVersionId}
26+
/>
27+
);
28+
};
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// rule: no-derived-state-effect
2+
// weakness: wrapper-transparency
3+
// source: React Bench fix-react-rdh-sofn-xyz-mailing-settings Jp9zWjq and Mf8qhaw
4+
5+
import { useEffect, useState } from "react";
6+
7+
interface ApiKey {
8+
id: string;
9+
createdAt: string;
10+
}
11+
12+
const sortApiKeys = (apiKeys: ApiKey[]): ApiKey[] =>
13+
[...apiKeys].sort(
14+
(firstApiKey, secondApiKey) =>
15+
new Date(secondApiKey.createdAt).getTime() - new Date(firstApiKey.createdAt).getTime(),
16+
);
17+
18+
export const Settings = ({ initialApiKeys }: { initialApiKeys: ApiKey[] }) => {
19+
const [apiKeys] = useState(initialApiKeys);
20+
const [apiKeyRows, setApiKeyRows] = useState<string[]>([]);
21+
22+
useEffect(() => {
23+
setApiKeyRows(sortApiKeys(apiKeys).map((apiKey) => apiKey.id));
24+
}, [apiKeys]);
25+
26+
return <output>{apiKeyRows.join(",")}</output>;
27+
};

0 commit comments

Comments
 (0)