next-drupal icon indicating copy to clipboard operation
next-drupal copied to clipboard

Adopt Next's app router

Open lorenzo-dallamuta opened this issue 1 year ago • 7 comments

Package

next-drupal (NPM package)

Describe the feature request

Many Next Drupal methods rely on the context object that the page router provides to getStaticProps.

According to the typings included in the Next package a context is an object with the shape:

type GetStaticPropsContext<
    Params extends ParsedUrlQuery = ParsedUrlQuery,
    Preview extends PreviewData = PreviewData
> = {
    params?: Params
    preview?: boolean
    previewData?: Preview
    draftMode?: boolean
    locale?: string
    locales?: string[]
    defaultLocale?: string
}

Preview data is also defined in the Next package as:

type PreviewData = string | false | object | undefined

ParsedUrlQuery is defined in the Node package as such (it's basically a Record<string, string[]>):

interface ParsedUrlQuery extends NodeJS.Dict<string | string[]> {}

Then in methods such as getResourceFromContext Next Drupal states that expect params as JsonApiParams:

type JsonApiParams = Record<string, any>

In other words: it should be doable for the user to "recreate their own context" as a stopgap solution.


So far I was able to recreate a context object with the utilities provided by the app router version of Next, so for example I would take the slug or the locale from the parameters of the main component function or check if draft mode is active by importing the draftMode function from next/headers.

/app/[...slug]/page.jsx

export default async function Page({ params: { lang, slug } }) {
    // for each required feature of the context interface the value has to be filled in
    // all other entries can be set ad undefined
    const context = { locale: lang, params: { slug } } features
    const path = await drupal.translatePathFromContext(context)
    const node = await drupal.getResourceFromContext(path, context)
    return ( ... )
}

I believe there's a wider discussion to be had for Next Drupal: most of the "context methods" internally call one of the non-context variants, with inputs enriched by handling the context object.

This was a desirable solution when the context object was provided to the user by the Next framework, but this is not the case anymore.

Broadly speaking, I see two possible paths to solve this issue (I'm partial to the second one):

  • provide an utility to recreate a context object (which is complicated since this would need to be done on each route and the necessary data isn't available from a single source)
  • move away from using a context object and have the distinct entries of the context object become method parameters (this still requires the user to access each relevant data source and pass it with something like an options object)

lorenzo-dallamuta avatar Jan 25 '24 09:01 lorenzo-dallamuta

The context object is removed from app router architecture and so agreeably, we should move away from dependence on methods that require context too. Those methods are essentially the "Pages" router support.

Fortunately, much of next-drupal already contains context-less methods. I wanna share a method I wrote that, with a bit of work, could be useful for next-drupal for App router support. https://gist.github.com/fiasco/00625053156c37f9f478721cd313e140

Some features of this method:

  • Given a pathname, you get the Drupal node object - no other context needed. You do have to pass a Drupal DrupalJsonApiParams object so the right fields are fetched for your node object.
  • It doesn't rely on subrequests because they're POST requests and are not cacheable. This does mean more requests are made to Drupal however.
  • It gives back the translated path to check for redirects
  • It will provide both the published and unpublished copies of a node in DraftMode so you can render HTML diffs with libraries like https://www.npmjs.com/package/htmldiff-js
  • While navigating in DraftMode, not all content has a draft state, so the hasDraft boolean makes it easier to make rendering decisions in DraftMode based on whether a revision is available or not.

I've noticed that my react server components are not caching the way I want to yet and I think this is related to how next-drupal fetches content (nextjs cache-control headers are directly tied to how fetch() calls are made during the page load).

We're using this method to provide toggle-able view modes in DraftMode between published, draft and HTML diff views.

We also had to implement our own methods and API routes to enable and end DraftMode. I can share those if they're helpful.

fiasco avatar Jan 25 '24 18:01 fiasco

I've noticed that my react server components are not caching the way I want to yet and I think this is related to how next-drupal fetches content (nextjs cache-control headers are directly tied to how fetch() calls are made during the page load).

Did you try to pass a custom fetcher to the DrupalClient instance? Something like this:

export const drupal = new DrupalClient(
  process.env.NEXT_PUBLIC_DRUPAL_BASE_URL!,
  {
    fetcher: (
      input: string | URL | globalThis.Request,
      init?: RequestInit,
    ): Promise<Response> => {
      return fetch(input, {
        cache: "no-store", // either this
        next: { revalidate: getGlobalCacheDuration() }, .. // or this
        ...init,
      })
    },
  },
)

lorenzo-dallamuta avatar Jan 26 '24 08:01 lorenzo-dallamuta

Yes, I set the next.revalidate to 3600 but haven't had success with that yet. It worked fine with other pages in the site that rely on S3 json resources but not with the Drupal responses which make me think it might be related to how Drupal is responding.

fiasco avatar Jan 27 '24 03:01 fiasco

I with with settings the dynamic constant to "force-static" which solved the issue

fiasco avatar Jan 30 '24 01:01 fiasco

Yes, I set the next.revalidate to 3600 but haven't had success with that yet. It worked fine with other pages in the site that rely on S3 json resources but not with the Drupal responses which make me think it might be related to how Drupal is responding.

Not sure if this applies, but atm time based revalidation has some issues with next. For me setting no-store for the fetcher function of next drupal and place its transactions inside some dynamic_cache worked to have them invalidate manually (it also worked for time based but only in dev environment).

Revalidation not working in deployment mode #5623

lorenzo-dallamuta avatar Jan 31 '24 08:01 lorenzo-dallamuta

The Pages Router-specific methods in DrupalClient are:

  • getResourceCollectionFromContext(): The App Router equivalent is getResourceCollection()
  • getResourceFromContext(): The App Router equivalent is getResource(type, uuid) or getResourceByPath(path)
  • getSearchIndexFromContext(): The App Router equivalent is `getSearchIndex()
  • translatePathFromContext(): The App router equivalent is translatePath()
  • getPathFromContext(): This is a helper function used by context-based methods and also by getResourceByPath() with a constructed context object. The parameters should be refactored and the method renamed.
  • getStaticPathsFromContext() and alias getPathsFromContext(): needs an App Router equivalent for the new generateStaticParams() function
    • buildStaticPathsFromResources(): This is a helper function for getStaticPathsFromContext().
    • buildStaticPathsParamsFromPaths(): This is a helper function for the helper function buildStaticPathsFromResources().
  • getAuthFromContextAndOptions(): This is just a helper function for context-based methods and doesn't need an App Router equivalent.

The example-router-migration is a new example that has been added to this repo that shows how to have full App Router support, but it uses a fake context parameter with getStaticPathsFromContext() and that should be replaced with a new generateStaticParams() method.

So it looks like we just need to have 2 new methods to fix this issue.

JohnAlbin avatar Feb 21 '24 17:02 JohnAlbin

PR #716 adds two new methods:

constructPathFromSegment()

This is the refactored getPathFromContext(). It returns a Drupal path given an array of Next.js path segments, path prefix and locale.

getResourceCollectionPathSegments()

This method supports Next.js' generateStaticParams() function. It returns an array of Drupal path-related data that can be used to provide a static list for use in Next.js Dynamic Segments.

Each item in the returned array looks like this:

{
  path: "/blog/some-category/a-blog-post", // The unaltered Drupal path alias
  type: "node--article",
  locale: "en", // or `undefined` if no `locales` requested.
  segments: ["blog", "some-category", "a-blog-post"],
}

In many cases, the segments array will the only item needed. But the other properties of the object will allow more advanced use-cases.

Examples:

When used in a app/[...slug]/page.tsx file, the [...slug] Dynamic Segment indicates that generateStaticParams() is expected to return an array of objects with a slug property containing an array of strings.

export async function generateStaticParams(): Promise<NodePageParams[]> {
  const resources = await drupal.getResourceCollectionPathSegments(
    ["node--page", "node--article"]
  );

  return resources.map((resource) => {
    return {
      slug: resource.segments,
    }
  });
}

When used in a app/[lang]/blog/[category]/[...slug]/page.tsx file, generateStaticParams() is expected to return an array of objects with a lang string, a category string and a slug array of strings.

export async function generateStaticParams(): Promise<NodePageParams[]> {
  const resources = await drupal.getResourceCollectionPathSegments(
   ["node--article"],
    {
      // The pathPrefix will be removed from the returned path segments array.
      pathPrefix: "/blog",
      // The list of locales to return.
      locales: ["en", "es"],
      // The default locale.
      defaultLocale: "en",
    }
  );

  return resources.map((resource) => {
    // NOTE: Because `pathPrefix` was set to "/blog",
    // "blog" will not be included in the segments array.

    // Grab the first item from the segments array to get
    // the needed "category" param.
    const category = resource.segments.unshift();

    return {
      lang: resource.locale,
      category,
      slug: resource.segments, 
    };
  })
}

JohnAlbin avatar Mar 08 '24 17:03 JohnAlbin