View transitions: `transitionEnabledOnThisPage` returns `true` when fallback is set to `none`

bholmesdev commented 1 month ago

Astro Info

Astro                    v4.11.5
Node                     v20.13.1
System                   macOS (arm64)
Package Manager          pnpm
Output                   hybrid
Adapter                  @astrojs/netlify
Integrations             @astrojs/markdoc
                         astro-icon
                         @astrojs/react
                         simple-stack-query

If this issue only occurs in one browser, which browser is a problem?

Firefox, Safari

Describe the Bug

I built a library that reruns a user's script whenever view transitions are enabled. I toggle this behavior using transitionEnabledOnThisPage from astro:transitions/client. However, I noticed this returns true even when fallback is set to none and my browser does not support view transitions.

What's the expected result?

I am using transitionEnabledOnThisPage to determine whether the astro:page-load event will fire. Since astro:page-load does not fire when the fallback is set to none, I would expect transitionEnabledOnThisPage to return false.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-gcu7qh?file=src%2Fpages%2Findex.astro

Participation

[X] I am willing to submit a pull request for this issue.

martrapp commented 1 month ago

Hi @bholmesdev, yes, in retrospective one might expect that ;-) I'm a bit reluctant to change the semantics of the API but on the other hand, a) I do not think that that this function is widely used and b) one might argument that it is a bug

Would be happy to see your PR on this! Should we defer it to 5.0?

In the meanwhile you could extend your test with && ... getFallback() !== 'none')

bholmesdev commented 1 month ago

I was unaware we had a getFallback()! I queried for the meta tag directly as a workaround. That should also suffice. Don't feel too strongly, but I do think "transition enabled" means "SPA routing" for my use cases.

If I were to make a PR, it would just add that getFallback() check. I can raise as a 5.0 item.

martrapp commented 1 month ago

Yes, please! Regarding the original intend you could talk to Matthew.

Inside the client side router it is not used as "SPA-mode" but "The client-side router code is available on that page in case we need to reload".

I skimmed our uses and it looks like adding getFallback() !== "none" wouldn't hurt, but you should check it thoroughly. I'm afraid we do not have e2e tests that check consequences in the behavior when transitionEnabledOnThisPage changes. 🙄

Alternatively, you could add a new function along the lines of "SPA routing".

withastro / astro