solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.1k stars 137 forks source link

Route load results in inconsistent revalidation behaviour #407

Closed katywings closed 2 months ago

katywings commented 2 months ago

Describe the bug

If a route has a load function that preloads cache x, not-preloaded cache y will have an inconsistent revalidation behavior (❌):

Behaviour with route load function:

Nr Action declaration Explicit revalidate x + y y is revalidated Behaviour is correct
1. action(serverFunction) No No
2. action(serverFunction) Yes Yes
3. action(() => serverFunction()) No Yes
4. action(() => serverFunction()) Yes Yes

Behaviour without route load function:

Nr Action declaration Explicit revalidate x + y y is revalidated Behaviour is correct
1. action(serverFunction) No Yes
2. action(serverFunction) Yes Yes
3. action(() => serverFunction()) No Yes
4. action(() => serverFunction()) Yes Yes

Your Example Website or App

Solid-Start with Basic Template

Steps to Reproduce the Bug or Issue

  1. Create a solid-start project (basic)
  2. pnpm install && pnpm dev
  3. Replace the about.tsx code with the code bellow:
  4. Press the buttons. Button 1. correctly updates x and y
  5. Set preloadX in the code to true and save
  6. Refresh the browser tab (Ctrl + R)
  7. Press the buttons. Button 1. now does not update y
import { Title } from "@solidjs/meta";
import { action, cache, createAsync, useAction, reload } from "@solidjs/router";

const getX = cache(async () => {
  "use server";
  return Math.random().toFixed(5);
}, "preload");

const getY = cache(async () => {
  "use server";
  return Math.random().toFixed(5);
}, "no-preload");

const update$ = async (explicit = false) => {
  "use server"
  if (explicit) {
    throw reload({
      revalidate: [getY.keyFor(), getX.keyFor()],
    });
  }
};

const update = action(update$);
const updateWrapped = action((args?) => update$(args))

const preloadX = false;

const buttonStyle = "font-size:1.5rem;padding:0.1rem 0.9rem;"

export const route = {
  load: async () => {
    preloadX && await getX();
  },
};

export default function Home() {
  const x = createAsync(() => getX());
  const y = createAsync(() => getY());

  const myReload = useAction(update);
  const myReloadWrapped = useAction(updateWrapped);

  return (
    <main>
      <Title>About</Title>
      <h1>Preload X: {preloadX + ""}</h1>
      <div style="display: flex; gap: 1rem; justify-content: center;">
        <button style={buttonStyle} onClick={() => myReload()}>
          1. Just Update
        </button>
        <button style={buttonStyle} onClick={() => myReload(true)}>
          2. Explicit revalidate
        </button>
        <button style={buttonStyle} onClick={() => myReloadWrapped()}>
          3. wrapped server function
        </button>
        <button style={buttonStyle} onClick={() => myReloadWrapped(true)}>
          4. Wrapped server function & explicit revalidate
        </button>
      </div>
      <pre style="font-size:2rem;">
        x: {x()}
        {"\n"}
        y: {y()}
      </pre>
    </main>
  );
}

Expected behavior

Preloading the cache x in a route load function, should not break the implicit revalidation of cache y.

Case 1. from the table above should always revalidate x and y.

Screenshots or Videos

https://github.com/solidjs/solid-router/assets/4012401/a0926f54-1f46-4917-9653-790a136d1a42

Platform

Additional context

No response

ryansolid commented 2 months ago

Thanks for the detailed bug report. I see the issue. It's interesting. With singleflight it assumes it does the fetching on the server for all invalidation. So it not being in the load function doesn't work. I didn't think of this. I was concerned with new things on forward navigation which do work. But I'm not sure how to make this one work.. maybe we need to include the invalidate keys on the response and that way if there are none we don't be so strict.. It's interesting.

EDIT: Oh wait we do know that. Ok let me see.

katywings commented 2 months ago

Thank you a lot for the fix!