solidjs / solid-router

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

Fix no-revalidate from server action #414

Closed LexSwed closed 4 months ago

LexSwed commented 5 months ago

When action returns

  return json(data, { revalidate: [] })

Empty revalidation keys array is serialised into an empty string, so response.headers.get("X-Revalidate") === ''. This makes

https://github.com/solidjs/solid-router/blob/57847624fb673003f6f0d8261ca7352ff00e7480/src/data/action.ts#L152

to be an array of [''] (Playground), causing all cache keys to be revalidated.

This PR adds empty string check, defaulting to an empty array, as empty array seem to correctly stop revalidation from happening.

Very subtle behaviour that should be tested, but I couldn't find a setup for actions testing.

changeset-bot[bot] commented 5 months ago

🦋 Changeset detected

Latest commit: 415e027d04084e8a864499a564afe1be37dc6385

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | --------------- | ----- | | @solidjs/router | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

LexSwed commented 5 months ago

I might have changed something else for this to start working, can't confirm on reproduction

LexSwed commented 5 months ago

It's actually my usage of actions with all the body being on the server, makes the response data not Response, skipping the check altogether.

https://github.com/solidjs/solid-router/blob/57847624fb673003f6f0d8261ca7352ff00e7480/src/data/action.ts#L150

This makes the situation with server/not server split quite complex to grasp. When running an action, cache that needs to be revalidated lives on the client. But if all action logic is on the server, handleResponse can't know which fields need to be invalidated.

LexSwed commented 4 months ago

Not fixing the issue I had, it was a different problem.