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

Route load results in inconsistent revalidation behaviour #407

Closed
katywings opened this issue Apr 22, 2024 · 2 comments
Closed

Route load results in inconsistent revalidation behaviour #407

katywings opened this issue Apr 22, 2024 · 2 comments

Comments

@katywings
Copy link

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

huch-2024-04-22_20.50.40.mp4

Platform

  • OS: Linux
  • Browser: Firefox
  • @solidjs/meta 0.29.3
  • @solidjs/router 0.13.2
  • @solidjs/start 1.0.0-rc.0
  • solid-js 1.8.16
  • vinxi 0.3.11

Additional context

No response

@ryansolid
Copy link
Member

ryansolid commented Apr 29, 2024

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
Copy link
Author

Thank you a lot for the fix!

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

No branches or pull requests

2 participants