Skip to content

Commit 91ee2db

Browse files
committed
Show reactivity warning on reactive variables in context provider value prop.
1 parent c842390 commit 91ee2db

File tree

2 files changed

+65
-15
lines changed

2 files changed

+65
-15
lines changed

src/rules/reactivity.ts

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -753,22 +753,40 @@ const rule: TSESLint.RuleModule<MessageIds, []> = {
753753
}
754754
};
755755
if (node.type === "JSXExpressionContainer") {
756-
// Expect a function if the attribute is like onClick={} or on:click={}.
757-
// From the docs: Events are never rebound and the bindings are not
758-
// reactive, as it is expensive to attach and detach listeners. Since
759-
// event handlers are called like any other function each time an event
760-
// fires, there is no need for reactivity; simply shortcut your handler
761-
// if desired.
762-
// What this means here is we actually do consider an event handler a
763-
// tracked scope expecting a function, i.e. it's okay to use changing
764-
// props/signals in the body of the function, even though the changes
765-
// don't affect when the handler will run.
766-
const expect =
756+
if (
767757
node.parent?.type === "JSXAttribute" &&
768758
/^on[:A-Z]/.test(sourceCode.getText(node.parent.name))
769-
? "called-function"
770-
: "expression";
771-
pushTrackedScope(node.expression, expect);
759+
) {
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
765+
// 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.
770+
pushTrackedScope(node.expression, "called-function");
771+
} else if (
772+
node.parent?.type === "JSXAttribute" &&
773+
node.parent.name.name === "value" &&
774+
node.parent.parent?.type === "JSXOpeningElement" &&
775+
((node.parent.parent.name.type === "JSXIdentifier" &&
776+
node.parent.parent.name.name.endsWith("Provider")) ||
777+
(node.parent.parent.name.type === "JSXMemberExpression" &&
778+
node.parent.parent.name.property.name === "Provider"))
779+
) {
780+
// From the docs: "The value passed to provider is passed to useContext as is. That means
781+
// wrapping as a reactive expression will not work. You should pass in Signals and Stores
782+
// directly instead of accessing them in the JSX."
783+
// For `<SomeContext.Provider value={}>` or `<SomeProvider value={}>`, do nothing, the
784+
// rule will warn later.
785+
// TODO: add some kind of "anti- tracked scope" that still warns but enhances the error
786+
// message if matched.
787+
} else {
788+
pushTrackedScope(node.expression, "expression");
789+
}
772790
} else if (node.type === "JSXSpreadAttribute") {
773791
// allow <div {...props.nestedProps} />; {...props} is already ignored
774792
pushTrackedScope(node.argument, "expression");

test/rules/reactivity.test.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,20 @@ export const cases = run("reactivity", rule, {
181181
const [signal, setSignal] = createSignal(1);
182182
return <div on:click={() => console.log(signal())} />;
183183
}`,
184-
// Don't warn on using props.initial* for initialization
184+
// Pass reactive variables as-is into provider value prop
185+
`const Component = props => {
186+
const [signal] = createSignal();
187+
return <SomeContext.Provider value={signal}>{props.children}</SomeContext.Provider>;
188+
}`,
189+
// Don't warn on using props.initial* or props.default* for initialization
185190
`function Component(props) {
186191
const [count, setCount] = useSignal(props.initialCount);
187192
return <div>{count()}</div>;
188193
}`,
194+
`function Component(props) {
195+
const [count, setCount] = useSignal(props.defaultCount);
196+
return <div>{count()}</div>;
197+
}`,
189198
// Store getters
190199
`const [state, setState] = createStore({
191200
firstName: 'Will',
@@ -557,6 +566,29 @@ export const cases = run("reactivity", rule, {
557566
},
558567
],
559568
},
569+
// provider value passed as-is
570+
{
571+
code: `
572+
const Component = props => {
573+
return <SomeContext.Provider value={props.value}>{props.children}</SomeContext.Provider>;
574+
}`,
575+
errors: [{ messageId: "untrackedReactive", data: { name: "props.value" } }],
576+
},
577+
{
578+
code: `
579+
const Component = props => {
580+
return <SomeProvider value={props.value}>{props.children}</SomeProvider>;
581+
}`,
582+
errors: [{ messageId: "untrackedReactive", data: { name: "props.value" } }],
583+
},
584+
{
585+
code: `
586+
const Component = props => {
587+
const [signal] = createSignal();
588+
return <SomeContext.Provider value={signal()} someOtherProp={props.foo}>{props.children}</SomeContext.Provider>;
589+
}`,
590+
errors: [{ messageId: "untrackedReactive", data: { name: "signal" } }],
591+
},
560592
// getOwner/runWithOwner
561593
{
562594
code: `

0 commit comments

Comments
 (0)