InitialData interfering with HMR on inline sql changes

[smilingthax]

Steps To Reproduce

1. Create index.md with inline sql query returning multiple rows and display results (e.g. chart, datatable, "show sql", ...).

2. Start dev server, open page in browser

• Make sure that /api/[...]/[...]/all-queries.json is correctly loaded
• the corresponding QueryStore should now have .opts.initialData as array w/ the expected length (= number of rows).

3. In index.md add (e.g.) LIMIT 1 to reduce the number of rows. HMR will reload the page in the browser. The page might scroll to the top (#1341 seems to be somewhat related).

Environment

• Node version (node -v): v20.11.1
• Evidence version: v29.0.2

Expected Behavior

The displayed results shows (e.g.) only a single row.

Actual Behaviour

The SQL Query text changes ("Show SQL"), but the number of rows stays unchanged (= multiple rows).

Reloading the whole page manually does finally display the correct result, though.

Workarounds

When starting the dev server BEFORE creating the page, and then navigating to it (needs more than 1 page), SQL changes directly lead to correct results after HMR. Also, AFAICT #1341 scroll-to-top seems to happen less frequently...

Alternatively, adding opts.initialDataDirty = true unconditionally, e.g. here: https://github.com/evidence-dev/evidence/blob/next/packages/query-store/src/QueryStore.ts#L233 "fixes" the problem.

(I believe there are additional cases where all-queries.json is not loaded, which do work correctly.)

Explanation

• HMR (on the server side) does not seem to notice the inline sql changes – it definitely doesn't act on them.

• This means: all-queries.json stays unchanged, and the client only loads app.css and +page.md via the HMR trigger.

• The injected svelte code in +page.md, https://github.com/evidence-dev/evidence/blob/next/packages/lib/preprocess/src/process-queries.cjs#L112 relies on the sql block name (id) to obtain the initialData,

• which ultimately comes fromgetPrerenderedQueries() (e.g. https://github.com/evidence-dev/evidence/blob/next/sites/example-project/src/pages/%2Blayout.js#L47), which is also not called again; but even if it would, it uses all-queries.json, which is still unchanged on the server.

• Thus, the old (wrong) [hash].arrow-Data is used again, leading to the observed lost sync between sql query text and sql query result.

• When creating the page after starting the dev server, the new page's queries are never added to the pre-rendered result cache, leading to empty initialData; the client-side obtained result against the parquet file are always correct, before and after HMR.

• https://github.com/evidence-dev/evidence/blob/next/packages/lib/preprocess/src/process-queries.cjs#L93 includes (basically) _${id}_changed = _${id}_current_query !== _${id}_initial_query to force empty initialData, but this does not capture the case at hand – and, according to the comments there, wasn't meant to.

• And finally, QueryStore's backgroundFetch, although executed, does not help, because it just throws away the (correct) result.

[smilingthax]

Ok, I now believe https://github.com/evidence-dev/evidence/commit/90e152cbc088e16d2f1d9686b25ba023322dc156 should have fixed this, but vite:afterUpdate just never triggers...

Version is vite/4.3.9 linux-x64 node-v20.11.1, coming from https://github.com/evidence-dev/template/blob/main/package-lock.json#L2142

Edit: npm install vite@5.1.6 does not seem to help, though...

[smilingthax]

Well, vite:afterUpdate does actually trigger, but _reactivity_manager's update() function https://github.com/evidence-dev/evidence/blob/next/packages/lib/preprocess/src/process-queries.cjs#L99 is not run again after the hmr.

Fix: Add __has_hmr_run as svelte dependency in https://github.com/evidence-dev/evidence/blob/next/packages/lib/preprocess/src/process-queries.cjs#L197

                // rerun if data changes during dev mode, likely source HMR, prevent initial for same reason as above
                let _${id}_hmr_once = false;
                $: if (dev) {
                    if (_${id}_hmr_once) {
                        data;
                        __has_hmr_run;      // <-- add dependency
                        _${id}_debounced_updater();
                    } else {
                        _${id}_hmr_once = true;
                    }
                }

(This does not improve the situation w/ https://github.com/evidence-dev/evidence/issues/1341 any more, though.)

[ItsMeBrianD]

@kwongz please give this a look to see if it is still occuring