SolidJS•9mo ago

Need clarity on eslint(solid/reactivity) warning use-case

I need help understanding how to deal with this ESLint warning when I am building a custom SolidJS hook library that doesn't have any visual components. There is a portion of my custom hook code that gets called out by this warning and I don't know how I should properly resolve it (or if it is safe to ignore it). The code snippet where I receive the warning is the following:

import { Accessor, createSignal, onMount, onCleanup } from "solid-js";

type LiveQueryResult = {
  readonly docs: Doc[];
  readonly rows: any[];
};

// NOTE: database is an accessor defined outside of this custom hook (via createMemo). createLiveQuery is effectively the inner function of an outer closure.
const createLiveQuery = (key: string, query = {}, initialRows: any[] = []): Accessor<LiveQueryResult> => {
    const [result, setResult] = createSignal({
      rows: initialRows,
      docs: initialRows.map((r) => r.doc),
    });

    const refreshRows = async () => {
      const res = await database().query(key, query);
      setResult({ ...res, docs: res.rows.map((r) => r.doc) });
    };

    onMount(() => {
      // this is where I receive the ESLint warning. Specifically on the callback passed to subscribe as the `refreshRows` function has internal reactivity due to using database() under the hood.
      const unsubscribe = database().subscribe(() => void refreshRows());

      onCleanup(() => {
        unsubscribe();
      });
    });

    return result;
  };

import { Accessor, createSignal, onMount, onCleanup } from "solid-js";

type LiveQueryResult = {
  readonly docs: Doc[];
  readonly rows: any[];
};

// NOTE: database is an accessor defined outside of this custom hook (via createMemo). createLiveQuery is effectively the inner function of an outer closure.
const createLiveQuery = (key: string, query = {}, initialRows: any[] = []): Accessor<LiveQueryResult> => {
    const [result, setResult] = createSignal({
      rows: initialRows,
      docs: initialRows.map((r) => r.doc),
    });

    const refreshRows = async () => {
      const res = await database().query(key, query);
      setResult({ ...res, docs: res.rows.map((r) => r.doc) });
    };

    onMount(() => {
      // this is where I receive the ESLint warning. Specifically on the callback passed to subscribe as the `refreshRows` function has internal reactivity due to using database() under the hood.
      const unsubscribe = database().subscribe(() => void refreshRows());

      onCleanup(() => {
        unsubscribe();
      });
    });

    return result;
  };

What is the proper way to address the warning in this case? Or is this something I can safely ignore perhaps?

3 Replies

Brendonovich•9mo ago

I think the problem eslint is trying to point out is that if database changes, refreshRows will use the new database but you'll still be subscribed to the old one. Doing something like this may be better:

const refreshRows = async (database: Database) => {
  const res = await database.query(key, query);
  setResult({ ...res, docs: res.rows.map((r) => r.doc) });
};

createEffect(() => {
  const db = database();
  const unsubscribe = db.subscribe(() => void refreshRows(db));

  onCleanup(() => {
    unsubscribe();
  });
});

const refreshRows = async (database: Database) => {
  const res = await database.query(key, query);
  setResult({ ...res, docs: res.rows.map((r) => r.doc) });
};

createEffect(() => {
  const db = database();
  const unsubscribe = db.subscribe(() => void refreshRows(db));

  onCleanup(() => {
    unsubscribe();
  });
});

You could use database() inside refreshRows but eslint might still warn you that you're using a memo outside of a reactive context, at least this way refreshRows will definitely use the same database value as the effect

MaveriX89•9mo ago

Ah, extracting it out into a local variable and injecting it into refreshRowsis a genius move and fixes the warning! Thank you so much for this insight and context! Super helpful! I feel like this use-case should be documented in the docs for that eslint(solid-reactivity) rule.

Brendonovich•9mo ago

Do note that I used createEffect instead of onMount too - otherwise the subscription won't be recreated when database changes

Gaming

Programming

Need clarity on eslint(solid/reactivity) warning use-case