Skip to content

Commit e034e2d

Browse files
authored
Merge pull request #44 from solidjs-community/fix/callback-refs
Silence reactivity warnings in callback refs.
2 parents 31e9cae + 4f89d3c commit e034e2d

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
@@ -200,6 +200,29 @@ export const cases = run("reactivity", rule, {
200200
// function expression inside tagged template literal expression is tracked scope
201201
"css`color: ${props => props.color}`;",
202202
"html`<div>${props => props.name}</div>`;",
203+
// refs
204+
`function Component() {
205+
let canvas;
206+
return <canvas ref={canvas} />;
207+
}`,
208+
`function Component() {
209+
let canvas;
210+
return (
211+
<canvas ref={c => {
212+
canvas = c;
213+
}} />
214+
);
215+
}`,
216+
`function Component() {
217+
const [index] = createSignal(0);
218+
let canvas;
219+
return (
220+
<canvas ref={c => {
221+
index();
222+
canvas = c;
223+
}} />
224+
);
225+
}`,
203226
],
204227
invalid: [
205228
// Untracked signals

0 commit comments

Comments
 (0)