search

Shopify / shopify-app-template-remix

288 stars 117 forks source link

Link Prefetch Redirecting / Not authenticated correctly #386

Open mkromann opened 8 months ago

mkromann commented 8 months ago

Issue summary

It seems like Remix's prefetch functionality isn't working. Prefetched routes are not authenticated, and results in the user being redirect to the auth page when clicked. See the logs below.

10:57:03 │ remix │ [shopify-app/INFO] Authenticating admin request
10:57:03 │ remix │ [shopify-app/DEBUG] Missing or invalid shop, redirecting to login path | {shop: null}
10:57:03 │ remix │ GET /app/products/6706610864243?issue=product-not-active&_data=routes%2Fapp.products.%24product_id 204 - - 3.245 ms
10:57:10 │ remix │ [shopify-app/INFO] Authenticating admin request
10:57:10 │ remix │ [shopify-app/DEBUG] Missing or invalid shop, redirecting to login path | {shop: null}
10:57:10 │ remix │ GET /app/products/6637895778419?issue=product-not-active&_data=routes%2Fapp.products.%24product_id._index 204 - - 3.593 ms
10:57:10 │ remix │ [shopify-app/INFO] Authenticating admin request
10:57:10 │ remix │ [shopify-app/DEBUG] Missing or invalid shop, redirecting to login path | {shop: null}
10:57:10 │ remix │ GET /app/products/6637895778419?issue=product-not-active&_data=routes%2Fapp.products.%24product_id 204 - - 2.421 ms
10:57:13 │ remix │ GET /auth/login?_data=routes%2Fauth.login 200 - - 3.756 ms

Expected behavior

I expect to be taken to the prefetched route. And to work as detailed by Remix here: https://remix.run/docs/en/main/components/link#prefetch

Actual behavior

The user is redirected to the auth/login route.

Steps to reproduce the problem

  1. Set prefetch='intent/render/viewport' on a Link or NavLink.
  2. Trigger the prefetch (eg. by hovering the link if 'intent' is chosen).
  3. Click Link/NavLink.
byrichardpowell commented 8 months ago

Hey @mkromann 👋

Good catch and thanks for opening an issue. Sorry this isn't working as expected.

It would be nice to do something here, and I would like to, because performance is super important. I'm going to chat with the Remix team, and the Shopify apps team to see what we can do here and we'll get back to you.

I also want to share some context on why this is difficult.

Last time I looked:

  1. Remix uses <link rel="prefetch"/> to prefetch stuff.
  2. Remix does not allow the developer to control this request.

The problem there is that authenticate.admin(request) from shopify.server relies on either a URL param or a session token header to authenticate the request because cookies won't work in the iFrame. URL params may expire (and we have no ways to set a fresh one), and we have no way to set an authorization header.

Some possibilities here:

  1. We can chat with the Remix team to see if we can evolve Remix here. Last time we chatted, this was pretty hard, but maybe there is a simpler way.
  2. Maybe we can add something inside App Bridge, or inside the AppProvider from @shopify/shopify-app-remix. It would look for <link rel="prefetch"/> and perform a fetch request instead, which would warm the browsers cache.
  3. Maybe we can add a <Link/> component to @shopify/shopify-app-remix. It would take the same API as Remix's <Link/> component but would handle prefetching itself, ensuring there is a session token param.

Just to set expectations we have some missing API's and bugs we need to get to first, so it might be a while, but I do want to do something here.

mkromann commented 8 months ago

Thanks for the answer @byrichardpowell. It's definitely something that have me considering foregoing embedding our internal apps.

I am not deep into how this could be resolved but will the proposed Middleware help here? Guess not, as it affords to modify to incoming request. https://github.com/remix-run/remix/discussions/7642

byrichardpowell commented 8 months ago

Hey @mkromann

Totally understand wanting to provide the best experience possible, that's awesome, thank you 🙏

For context, I think non embedded apps come with their own UX tradeoffs like forgoing App Bridge components and the Shopify navigation. We'll look into the preloading issue though.

rikbrown commented 7 months ago

Just curious if there's any rough timeline on supporting this? Thank you!

Maybe we can add a component to @shopify/shopify-app-remix. It would take the same API as Remix's component but would handle prefetching itself, ensuring there is a session token param.

Any pointers to how one can generate the session token to do this themselves?

Johnpaulk94 commented 3 months ago

any updates on this issue?