Skip to content

Commit 4f89d3c

Browse files
committed
Silence reactivity warnings in callback refs.
1 parent 47567ec commit 4f89d3c

File tree

2 files changed

+41
-11
lines changed

2 files changed

+41
-11
lines changed

src/rules/reactivity.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -236,13 +236,13 @@ const rule: TSESLint.RuleModule<MessageIds, []> = {
236236
messages: {
237237
noWrite: "The reactive variable '{{name}}' should not be reassigned or altered directly.",
238238
untrackedReactive:
239-
"The reactive variable '{{name}}' should be used within JSX, a tracked scope (like createEffect), or inside an event handler function. Details: https://github.com/solidjs-community/eslint-plugin-solid/blob/main/docs/reactivity.md.",
239+
"The reactive variable '{{name}}' should be used within JSX, a tracked scope (like createEffect), or inside an event handler function.",
240240
expectedFunctionGotExpression:
241241
"The reactive variable '{{name}}' should be wrapped in a function for reactivity. This includes event handler bindings, which are not reactive like other JSX props.",
242242
badSignal:
243243
"The reactive variable '{{name}}' should be called as a function when used in {{where}}.",
244244
badUnnamedDerivedSignal:
245-
"This function should be passed to a tracked scope (like createEffect) or an event handler because it contains reactivity. Details: https://github.com/solidjs-community/eslint-plugin-solid/blob/main/docs/reactivity.md.",
245+
"This function should be passed to a tracked scope (like createEffect) or an event handler because it contains reactivity.",
246246
shouldDestructure:
247247
"For proper analysis, array destructuring should be used to capture the {{nth}}result of this function call.",
248248
shouldAssign:
@@ -757,16 +757,14 @@ const rule: TSESLint.RuleModule<MessageIds, []> = {
757757
node.parent?.type === "JSXAttribute" &&
758758
/^on[:A-Z]/.test(sourceCode.getText(node.parent.name))
759759
) {
760-
// Expect a function if the attribute is like onClick={} or on:click={}.
761-
// From the docs: Events are never rebound and the bindings are not
762-
// reactive, as it is expensive to attach and detach listeners. Since
763-
// event handlers are called like any other function each time an event
764-
// fires, there is no need for reactivity; simply shortcut your handler
760+
// Expect a function if the attribute is like onClick={} or on:click={}. From the docs:
761+
// Events are never rebound and the bindings are not reactive, as it is expensive to
762+
// attach and detach listeners. Since event handlers are called like any other function
763+
// each time an event fires, there is no need for reactivity; simply shortcut your handler
765764
// if desired.
766-
// What this means here is we actually do consider an event handler a
767-
// tracked scope expecting a function, i.e. it's okay to use changing
768-
// props/signals in the body of the function, even though the changes
769-
// don't affect when the handler will run.
765+
// What this means here is we actually do consider an event handler a tracked scope
766+
// expecting a function, i.e. it's okay to use changing props/signals in the body of the
767+
// function, even though the changes don't affect when the handler will run.
770768
pushTrackedScope(node.expression, "called-function");
771769
} else if (
772770
node.parent?.type === "JSXAttribute" &&
@@ -784,6 +782,15 @@ const rule: TSESLint.RuleModule<MessageIds, []> = {
784782
// rule will warn later.
785783
// TODO: add some kind of "anti- tracked scope" that still warns but enhances the error
786784
// message if matched.
785+
} else if (
786+
node.parent?.type === "JSXAttribute" &&
787+
node.parent.name.name === "ref" &&
788+
isFunctionNode(node.expression)
789+
) {
790+
// Callback/function refs are called when an element is created but before it is connected
791+
// to the DOM. This is semantically a "called function", so it's fine to read reactive
792+
// variables here.
793+
pushTrackedScope(node.expression, "called-function");
787794
} else {
788795
pushTrackedScope(node.expression, "expression");
789796
}

test/rules/reactivity.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,29 @@ export const cases = run("reactivity", rule, {
216216
// function expression inside tagged template literal expression is tracked scope
217217
"css`color: ${props => props.color}`;",
218218
"html`<div>${props => props.name}</div>`;",
219+
// refs
220+
`function Component() {
221+
let canvas;
222+
return <canvas ref={canvas} />;
223+
}`,
224+
`function Component() {
225+
let canvas;
226+
return (
227+
<canvas ref={c => {
228+
canvas = c;
229+
}} />
230+
);
231+
}`,
232+
`function Component() {
233+
const [index] = createSignal(0);
234+
let canvas;
235+
return (
236+
<canvas ref={c => {
237+
index();
238+
canvas = c;
239+
}} />
240+
);
241+
}`,
219242
],
220243
invalid: [
221244
// Untracked signals

0 commit comments

Comments
 (0)