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

Url resolution #17

Open
markomitranic opened this issue Apr 4, 2024 · 6 comments
Open

Url resolution #17

markomitranic opened this issue Apr 4, 2024 · 6 comments

Comments

@markomitranic
Copy link

Hey folks!

First off, amazing stuff you made here. If I got a penny for every half assed or overcomplicated implementation of pages in Sanity I’ve seen, I’d be a rich man. I like the simplicity and the opinionatedness.

I’m a tech lead at an agency much like your own, and am interested in contributing.

Anyway, I’ve tried using your preview on a project with i18n, and quickly noticed that your previews get the url by constructing it themselves, instead of exposing a method that would allow me to resolve URLs on my own. Or, perhaps an escape hatch exists and I was too dim to find it?

In my use case, for example, the customer has 24 i18n sets, spread across 16 domains :) meaning, we have to provide our own route resolution logic. Unfortunately intl is the one place where being opinionated doesn’t pay off.

@siffogh
Copy link
Contributor

siffogh commented Apr 6, 2024

@markomitranic Thank you for the kind words. You're right, localizePathname doesn't support such URLs.
Do you want to have a go at it? 1 quick thought I had would be to perhaps expose a custom localizePath option in both i18n object of both the plugin and definePathname.

@markomitranic
Copy link
Author

markomitranic commented Apr 7, 2024

Hey @siffogh absolutely, I'd love to be able to collaborate on this. If we can get it into shape, regarding url resolution, I'm pretty sure we'll see wider adoption (and contribution) from within my agency, I've already talked to other leads about this, and we'd love to help out.

I've been playing around with doing it locally, right now, but with limited success. The issue isn't that you don't expose an override for localizePath, that'd be easy to fix - i simply used a server action to fetch the url instead of using localizePath.

The real issue is actually that Sanity's own plugins, like presentation or iframe seem to ignore the domain name altogether and only forward the path to the draft API. I'm hoping that I've just missed something tho, so I'll try again tomorrow when I'm better rested :p

@markomitranic
Copy link
Author

markomitranic commented Apr 7, 2024

Opened sanity-io/visual-editing#1280 this morning, hoping to get some insight into this from the Sanity team.

For reference, the way I've done this thus far was manually generating my own draft API urls, and manually invoking them. Either as preview urls via productionUrl or yesterday by hijacking the preview button in PathnameFieldComponent... Not great.

import { createPreviewSecret } from "@sanity/preview-url-secret/create-secret";

/**
 * This is what we provide to `sanity.config.ts`.
 */
export const productionUrl: DocumentPluginOptions["productionUrl"] = async (
  prev: string | undefined,
  context: ResolveProductionUrlContext,
): Promise<string | undefined> => {
  const { document } = context;
  if (document._type === "page") {
    return generatePagePreviewUrl(context) ?? prev;
  }
  return prev;
};

/**
 * Returns a preview URL for a given "page" document, or undefined if not possible.
 * Doesn't return real URLs, instead wraps them in a `draftMode` request with a sanity token.
 */
async function generatePagePreviewUrl(context: ResolveProductionUrlContext) {
  const currentLocale = Locale.Danish_DK; // Studio is hosted here.
  const { getClient, document, currentUser } = context;
  const client = getClient({ apiVersion });

   // Resolve the real (possibly absolute) url with our own custom logic based on `next-intl`
  const resolvedUrl = await (async () => {
    const page = await client.fetch<{
      pathname: string | null;
      locale: SanityLocale | null;
    } | null>(
      `*[_type == 'page' && _id == $id][0] { "pathname": pathname.current, locale }`,
      { id: document._id },
    );
    if (!page?.pathname || !page?.locale) return undefined;
    const { pathname, locale } = page;
    return route({ type: "PAGE", currentLocale, locale, pathname }).href;
  })();

  const { secret } = await createPreviewSecret(
    client,
    "@sanity/presentation",
    typeof window === "undefined" ? "" : location.href,
    currentUser?.id,
  );

  // Draft URL Currently hardcoded for demo purposes, in reality can be on different domains, depending on the `resolvedUrl`.
  return constructDraftUrl("/api/draft/enable", secret, resolvedUrl).toString();
}

function constructDraftUrl(
  draftEndpoint: string,
  secret: string,
  resolvedUrl = "/",
) {
  const enablePreviewModeUrl = new URL(
    draftEndpoint,
    typeof location === "undefined" ? "https://localhost" : location.origin,
  );
  enablePreviewModeUrl.searchParams.set("sanity-preview-secret", secret);
  enablePreviewModeUrl.searchParams.set("sanity-preview-pathname", resolvedUrl ?? "/");
  return enablePreviewModeUrl;
}

@markomitranic
Copy link
Author

markomitranic commented Apr 8, 2024

Ok, to recap my intent, as this ticket will remain open for a while, until I can find someone in Sanity to tell us if they have interest in sanity-io/visual-editing#1280 being implemented.

If we can get @sanity/preview-url-secret to support absolute urls, we wouldn't have to change much in tinloof's package:

  • the preview should expose (if it doesn't already) whatever config options we implemented in @sanity/preview-url-secret.
  • the pathname and listitem components should not calculate their own urls, but instead just fetch the preview url from the context.
  • TBD: we'll also need to understand how to marry these changes with the custom pathing logic that tinloof's internationalization wrapper provides, In theory, we could just have tinloof's url construction be the default fallback, using the same properties, if an override isn't provided by the user.

If we can't get @sanity/preview-url-secret merged, I'll propose an alternative: we've confirmed that the iframe component works just fine with absolute urls, so I'll just suggest the same changes as above, plus an additional change that allows us to optionally circumvent @sanity/preview-url-secret using our own function in config, such as the one that I've posted above. Not ideal, but would offer A TON of freedom.

@siffogh
Copy link
Contributor

siffogh commented Apr 8, 2024

@markomitranic sounds like a plan, thank you for the additional details. I look forward to enabling this 🔥

@markomitranic
Copy link
Author

@siffogh there has been some update from Sanity side, and a commitment to introduce the changes I've proposed. We don't have an ETA yet, but I will update this issue when we do.

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