Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: What's a tracked scope? #8

Closed
imedadel opened this issue Jan 26, 2022 · 9 comments
Closed

Question: What's a tracked scope? #8

imedadel opened this issue Jan 26, 2022 · 9 comments
Assignees
Labels
question Further information is requested

Comments

@imedadel
Copy link

Hello Josh! I'm enjoying this ESLint plugin, however, I'm having trouble understanding the meaning of a tracked scope and the examples aren't quite answering my question. In my case, I wanted to pass a prop to a custom hook/function but I'm getting either The reactive variable 'localRef' should be used within a tracked scope or This function should be passed to a tracked scope because it contains reactivity.

Example:

const Component = (props) => {
  const [local, rest] = splitProps(props, ["ref"]);
  const localRef = () => local.ref;
  const composedRef1 = useComposedRefs(localRef); // error: The reactive variable 'localRef' should be used within a tracked scope
  const composedRef2 = useComposedRefs(() => local.ref); // error: This function should be passed to a tracked scope because it contains reactivity.
	// ...
}

What am I missing? 😩

And, sorry if this is not the right place to ask!

@joshwilsonvu
Copy link
Collaborator

joshwilsonvu commented Jan 26, 2022

Hi, thanks for the question, and I don't mind you asking here at all. Tracked scopes are mentioned a few times in the Solid docs; they're basically any function or expression where it's okay to have reactive variables because Solid is watching for changes. The idiomatic example is createEffect and JSX, but there's several Solid primitives that do this.

function Component() {
  const [signal, setSignal] = createSignal(1);
  setTimeout(() => {
    setSignal(2);
  }, 1000);
  createEffect(() => console.log(signal())); // tracked scope, logs 1, then 2 when signal changes

  return <div>{signal()}</div>; // tracked scope, displays 1, then 2 when signal changes
}

The reactivity rule tries to make sure that everywhere you use a reactive variable (signal, props, memos, etc., function containing any of these), Solid is able to watch for changes and do things like re-run effects and update HTML. Here's an example where this rule would warn because the console.log(signal()) is not in a tracked scope.

function Component() {
  const [signal, setSignal] = createSignal(1);
  setTimeout(() => {
    setSignal(2);
  });
  console.log(signal()); // only logs 1, never logs 2
}

I would have liked to link to an answer to you question in the error message, but couldn't find a good explanation.


Now, the reactivity rule is doing its best, but there are a lot of edge cases to uncover, and I think custom hooks is one of them. I could write a hook that works as a tracking scope (because it uses Solid primitives that create a tracking scope), but the rule would incorrectly warn.

const [signal] = createSignal(1);
function createEffectCopy(...args) {
  return createEffect(...args);
}
createEffectCopy(() => console.log(signal())); // warns, even though this is fine

There's not a perfectly accurate solution here: the reactivity rule can't know whether a custom hook creates a tracking scope or not. But I don't want to annoy the programmer, so the rule shouldn't automatically warn about using signals/props in a custom hook. You've discovered a bug!

Thanks for reading, hope that's helpful, and in a future release the rule won't warn when your hook is named use* or create*. I'll leave this issue open until that's fixed.

@imedadel
Copy link
Author

Thank you for the extremely detailed answer, this definitely helped a lot!

@joshwilsonvu
Copy link
Collaborator

No problem! For now, just disable the rule on each line with /* eslint-disable-line solid/reactivity */. If you add "reportUnusedDisableDirectives": true to your ESLint config, it will tell you when the disable comment isn't needed anymore.

@joshwilsonvu joshwilsonvu pinned this issue Jan 26, 2022
@millette
Copy link

Here's another example I'm having trouble with

https://github.com/millette/sobolid/blob/e827675473a2ef99bc8e1f21c762a1253e14b4d4/src/pages/credits.tsx#L77-L93

Specifically line 83. I'm also seeing the same warning with other <For> components.

Maybe it's late and I don't quite understand the rule, or maybe it's another edge case. I'd like to find out :-)

@joshwilsonvu
Copy link
Collaborator

joshwilsonvu commented Jan 31, 2022

Hi @millette, I double checked and I believe this case is actually reporting a bug. The offending code:

onClick={setSelected.bind(
  null,
  i() /* eslint-disable-line solid/reactivity */
)}

From the docs:

Events are never rebound and the bindings are not reactive, as it is expensive to attach and detach listeners.

Unlike React, where you render multiple times and .bind() creates a new function each render, your Solid component only runs once and the function you pass to onClick has to work by capturing i and calling it when needed.

In your code, setSelected is bound once to whatever the initial value of i is. In the example below, as the value in i changes over time, the event handler function gets the current value each time it runs.

onClick={() => setSelected(i())}

With Solid, there is no performance penalty to this code. The reactivity rule expects there to be a function here and lets you use signals and props within the function without warning.

Let me know if there's anything else I can help you with!

@millette
Copy link

@joshwilsonvu Thanks for looking into it. I checked the docs and also tried

<button onClick={[setSelected, i()]}>

That's still triggering the same warning.

When I use your suggestion, everything works and no warning. I believe the array version should also work without a warning.

@millette
Copy link

@joshwilsonvu
Copy link
Collaborator

I checked the array syntax and it doesn't seem to work in practice. In my example it always logs the initial value and never updates, which fits with "the bindings are not reactive" from the docs. I think the warning is correct: you have to use a function if you're using signals or props. The array syntax seems to be for non-reactive arguments, like constant strings/numbers/etc.

With your createResource example, you're right that the first argument can be reactive, but it has to be a signal variable that's not called (createResource(signal, ...)). Solid will call it to determine when changes occur. In your case you need a derived signal, a function returning the prop: createResource(() => props.partsFileName, parseIt). See the docs.

@joshwilsonvu
Copy link
Collaborator

@imedadel The issue with custom hooks should be resolved in v0.4.4. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants