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

Adopt Next's app router #665

Closed
lorenzo-dallamuta opened this issue Jan 25, 2024 · 7 comments
Closed

Adopt Next's app router #665

lorenzo-dallamuta opened this issue Jan 25, 2024 · 7 comments
Labels
area: next-drupal enhancement New feature or request

Comments

@lorenzo-dallamuta
Copy link

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 lorenzo-dallamuta added enhancement New feature or request triage A new issue that needs triage labels Jan 25, 2024
@fiasco
Copy link
Contributor

fiasco commented Jan 25, 2024

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.

@lorenzo-dallamuta
Copy link
Author

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,
      })
    },
  },
)

@fiasco
Copy link
Contributor

fiasco commented Jan 27, 2024

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
Copy link
Contributor

fiasco commented Jan 30, 2024

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

@lorenzo-dallamuta
Copy link
Author

lorenzo-dallamuta commented Jan 31, 2024

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

@JohnAlbin JohnAlbin added area: next-drupal and removed triage A new issue that needs triage labels Feb 20, 2024
@JohnAlbin JohnAlbin added this to the next-drupal 2.0.0 milestone Feb 20, 2024
@JohnAlbin
Copy link
Collaborator

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 added a commit that referenced this issue Mar 7, 2024
The DrupalClient class has been refactored to inherit from the NextDrupalFetch
(base class) and NextDrupal (JsonAPI/App Router class). DrupalClient class
contains the methods that are only needed to support Next.js' Pages Router.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Mar 8, 2024
The DrupalClient class has been refactored to inherit from the NextDrupalFetch
(base class) and NextDrupal (JsonAPI/App Router class). DrupalClient class
contains the methods that are only needed to support Next.js' Pages Router.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Mar 8, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Mar 8, 2024
@JohnAlbin
Copy link
Collaborator

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 added a commit that referenced this issue Mar 8, 2024
The DrupalClient class has been refactored to inherit from the NextDrupalFetch
(base class) and NextDrupal (JsonAPI/App Router class). DrupalClient class
contains the methods that are only needed to support Next.js' Pages Router.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Mar 8, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Mar 8, 2024
JohnAlbin added a commit that referenced this issue Mar 8, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Mar 8, 2024
JohnAlbin added a commit that referenced this issue Mar 10, 2024
The DrupalClient class has been refactored to inherit from the NextDrupalFetch
(base class) and NextDrupal (JsonAPI/App Router class). DrupalClient class
contains the methods that are only needed to support Next.js' Pages Router.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Mar 10, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Mar 10, 2024
JohnAlbin added a commit that referenced this issue Mar 10, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Mar 10, 2024
JohnAlbin added a commit that referenced this issue Mar 11, 2024
The DrupalClient class has been refactored to inherit from the NextDrupalFetch
(base class) and NextDrupal (JsonAPI/App Router class). DrupalClient class
contains the methods that are only needed to support Next.js' Pages Router.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Mar 11, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 1, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 1, 2024
JohnAlbin added a commit that referenced this issue Apr 2, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 2, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 2, 2024
JohnAlbin added a commit that referenced this issue Apr 2, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 2, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 2, 2024
JohnAlbin added a commit that referenced this issue Apr 2, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 2, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 2, 2024
JohnAlbin added a commit that referenced this issue Apr 13, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 13, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 13, 2024
JohnAlbin added a commit that referenced this issue Apr 14, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 14, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 14, 2024
JohnAlbin added a commit that referenced this issue Apr 18, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
JohnAlbin added a commit that referenced this issue Apr 18, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
JohnAlbin added a commit that referenced this issue Apr 18, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
…ticParams()

The App Router's new generateStaticParams() replaces Pages Router's
getStaticProps(). The new getResourceCollectionPathSegments() method supports
generateStaticParams() by returning a list of path segments. This new method
replaces the old getStaticPathsFromContext() method.

Fixes #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
JohnAlbin added a commit that referenced this issue Apr 18, 2024
The DrupalClient class has been renamed to NextDrupalPages and has been
refactored to inherit from the NextDrupalBase (base class) and NextDrupal
(JsonAPI/App Router class). NextDrupalPages class contains the methods that are
only needed to support Next.js' Pages Router. NextDrupalPages is also available
as "DrupalClient" for backwards-compatibility.

The getPathFromContext() method has been replaced with the
constructPathFromSegment() method for App Router usages.

Issue #665
JohnAlbin added a commit that referenced this issue Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: next-drupal enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants