solidjs / solid-docs-next

SolidJS Docs.
https://docs.solidjs.com/
208 stars 225 forks source link

[Content]: Usage of setInterval outside of onMount is harmful in SSR context #785

Open marvin-j97 opened 3 weeks ago

marvin-j97 commented 3 weeks ago

πŸ“š Subject area/topic

Components

πŸ“‹ Page(s) affected (or suggested, for new content)

https://docs.solidjs.com/ https://docs.solidjs.com/solid-router https://docs.solidjs.com/solid-start https://docs.solidjs.com/solid-meta

πŸ“‹ Description of content that is out-of-date or incorrect

All the landing pages in the Solid docs show off setInterval in a component's top level. I consider this to be an anti-pattern at best, and harmful at worst (in the context of SSR). With the release of Solid Start, I think this pattern should not be promoted anymore.

function Counter() {
  const [count, setCount] = createSignal(0);

  setInterval(() => setCount((prev) => prev + 1), 1000);

  return (
    <div>
      <p>Count: {count()}</p>
    </div>
  );
}

What happens when you run this code in Solid Start? Every render will create a new interval and eventually OOM your machine:

image

https://www.solidjs.com/ also has the same example component, however at least it clears the interval onCleanup. While still unnecessarily creating an interval on the server, at least it doesn't leak resources. IMO any use of setInterval should be moved it into an onMount with a onCleanup.

function Counter() {
  const [count, setCount] = createSignal(0);

  onMount((() => {
    const interval = setInterval(() => setCount((prev) => prev + 1), 1000);

    onCleanup(() => clearInterval(interval));
  }));

  return (
    <div>
      <p>Count: {count()}</p>
    </div>
  );
}

πŸ–₯️ Reproduction in StackBlitz (if reporting incorrect content or code samples)

No response