prismicio / prismic-client

The official JavaScript + TypeScript client library for Prismic
https://prismic.io/docs/technical-reference/prismicio-client
Apache License 2.0
169 stars 61 forks source link

fix: only use active preview refs #310

Closed lihbr closed 1 year ago

lihbr commented 1 year ago

Types of changes

Description

By default, the client automatically tries to fetch previewed content by using the preview cookie as a ref when one is available. However, in some cases, the preview cookie is present, but in an inactive state, leading to unexpected behavior.

The preview cookie can be in an inactive state when you're on your Prismic website, logged in to Prismic, but outside of a preview session: the toolbar loads, and sets an inactive preview cookie.

Inactive preview cookie example ![image](https://github.com/prismicio/prismic-client/assets/25330882/8498adf0-b687-4567-ae82-a96c7b8b62f4)

When the preview cookie is available, but in this state, it is still picked up by the client, as demonstrated in this Replit: https://replit.com/@lihbr/prismicioprismic-client309#index.js

Replit logs ![image](https://github.com/prismicio/prismic-client/assets/25330882/e4e682d4-6e9d-4cb7-985c-f808c230e681)

When the client uses this inactive preview cookie as a ref to query the API, the API falls back to the current master ref (odd, but alright).

This might sound convenient, however, because Cloudflare caches requests based on their URL, and because the master ref is implicit in this calls, this means subsequent API calls will always return the same, cached, response from the master ref that was used to build it. This means the content will be out of date as soon as a new master ref is available (i.e. something gets published)


This PR fixes the above by validating that the preview cookie is in an active state before leveraging it using the solution currently implemented by @prismicio/next: https://github.com/prismicio/prismic-next/blob/5c712675a14f6f504fe7914a73ceecb7ca430e4f/src/enableAutoPreviews.ts#L106-L122 (similar solution on Nuxt module: https://github.com/nuxt-modules/prismic/blob/217f4f15e304b5f8a61ece34f4476736bd38ae0a/src/runtime/plugin.ts#L46-L52)

This works OK, but has some corner cases:

A better solution would be to check for the key belonging to the repository the client instance is connected to, for it to be present and valid. However, checking for the repository name as a key in the preview cookie object is tough:

So, I don't know...

Resolves: https://github.com/nuxt-modules/prismic/issues/198

Checklist:

angeloashmore commented 1 year ago

The preview cookie can be in an inactive state when you're on your Prismic website, logged in to Prismic, but outside of a preview session: the toolbar loads, and sets an inactive preview cookie.

I believe the _tracker value is used to populate the Prismic Edit Button. The tracker fingerprints queries with the page's URL to pair a page with a set of Prismic documents. By removing this automatic behavior, we are essentially killing the Prismic Edit Button.

That being said, the Edit Button is already practically dead. Statically rendered website cannot use the Edit Button without performing hacks. We will ultimately need to re-think how the Edit Button works (which we already started in https://github.com/prismicio/prismic-toolbar/pull/95).

We should get approval to disable _tracker before merging this.

Only works for .prismic.io previews, doesn't work on staging (.wroom.io I assume, but the toolbar might be weird)

Should we move forward with this solution, we could simply add .wroom.io and any other known domain. The RegExp solution is simple and easy to understand.

Only works if you're only relying on one Prismic repository to build your website, might have unexpected behaviors if you're fetching data from two or more Prismic repositories

I'm not sure what happens when more than one Prismic toolbar is present on a site, one for each domain. Is the cookie subject to race conditions where it only contains the last repository? Or does it merge the cookies and contain every repository available?

In either case, .prismic.io should only exist if a preview session is active, but I don't know the toolbar's behavior to know for sure.

A better solution would be to check for the key belonging to the repository the client instance is connected to, for it to be present and valid. However, checking for the repository name as a key in the preview cookie object is tough:

We don't always have the repository name (we work internally with repository endpoints) We can't infer it safely (user might proxy their repository endpoint, or whatever)

In @prismicio/client, we usually call /api/v2 for every query, which means we usually have access to all repository metadata. We can mine some of that metadata, such as the forms or oauth_* properties, to retrieve the endpoint's repository name. That would work even for proxied URLs that don't contain the repository name.

However, we currently only fetch that endpoint if we need it. If the ref is sourced through different means, such as through a preview session or client.queryContentFromRef(), we don't send a request to /api/v2.

We could change that behavior to always request the repository metadata and know the repository endpoint. I think we should avoid that if possible since it technically would make some queries slower than they are today.

angeloashmore commented 1 year ago

Code-wise, this PR looks very clean! Let's consider the above points before moving forward, however. We may still end up merging this PR as it currently stands.

lihbr commented 1 year ago

Actually, looking at the toolbar code, the tracker should be refreshed on every load and page navigation: https://github.com/prismicio/prismic-toolbar/blob/b29f92787f931dcfe472666923190d376f3abf38/src/toolbar/prediction.js#L35

So I guess this is an edge case that only happens when:

I think this makes it a "wontfix"? Except if you have an idea to work that around? What do you think?

angeloashmore commented 1 year ago

Because this only happens to Prismic content writers and not to the general public, and only affects requests made at runtime with access to cookies (i.e. not on CI at build time for static sites), then we could simply tell Prismic developers and content writers to delete the cookie.

The logic in this PR, or some modified version of it, could still make a return if/when we decide to modify the toolbar's tracking behavior.

In the meantime, let's close the PR, unless you have any new ideas/thoughts on the topic.

lihbr commented 1 year ago

Alright, we decided not to move forward with the above and label it as "wontfix"

This is an edge case that affects only Prismic users under this scenario:

  1. A developer started developing a website with the toolbar on, it set a tracker
  2. At some point, the developer decided to remove the toolbar from their site, this made the tracker stale
  3. Because the tracker became stale, content fetched on their session would also remain stale

To get out of this "corrupted" state, simply delete your io.prismic.preview cookie in your browser. Content should then be fetched fresh again.