feat(gatsby): Match node manifest pages by page context slug

integration-tests/node-manifest/__tests__/create-node-manifest.test.js

@@ -70,6 +70,14 @@ describe(`Node Manifest API in "gatsby ${gatsbyCommandName}"`, () => {
     expect(manifestFileContents.foundPageBy).toBe(`context.id`)
   })
 
+  it(`Creates an accurate node manifest when ownerNodeId isn't present but there's a matching "slug" in pageContext`, async () => {
+    const manifestFileContents = await getManifestContents(5)
+
+    expect(manifestFileContents.node.id).toBe(`5`)
+    expect(manifestFileContents.page.path).toBe(`/slug-test-path`)
+    expect(manifestFileContents.foundPageBy).toBe(`context.slug`)
+  })
+
   if (gatsbyCommandName === `build`) {
     // this doesn't work in gatsby develop since query tracking
     // only runs when visiting a page in browser.

integration-tests/node-manifest/gatsby-node.js

@@ -4,7 +4,7 @@ const createManifestId = nodeId => `${commandName}-${nodeId}`
 
 exports.sourceNodes = ({ actions }) => {
   // template nodes
-  for (let id = 1; id < 5; id++) {
+  for (let id = 1; id < 6; id++) {
     const node = {
       id: `${id}`,
       internal: {
@@ -13,6 +13,10 @@ exports.sourceNodes = ({ actions }) => {
       },
     }
 
+    if (id === 5) {
+      node.slug = `test-slug`
+    }
+
     actions.createNode(node)
 
     actions.unstable_createNodeManifest({
@@ -108,4 +112,12 @@ exports.createPages = ({ actions }) => {
     path: `three-alternative`,
     component: require.resolve(`./src/templates/three.js`),
   })
+
+  actions.createPage({
+    path: `slug-test-path`,
+    context: {
+      slug: `test-slug`,
+    },
+    component: require.resolve(`./src/templates/four.js`),
+  })
 }

integration-tests/node-manifest/src/templates/four.js

@@ -0,0 +1,17 @@
+import { graphql } from "gatsby"
+import React from "react"
+
+export default function Four({ data }) {
+  return <div>Template 4. Node by slug {data.testNode.slug}</div>
+}
+
+export const query = graphql`
+  query SLUG_TEST($slug: String) {
+    testNode(slug: { eq: $slug }) {
+      id
+    }
+    otherNode: testNode(id: { eq: "2" }) {
+      id
+    }
+  }
+`

packages/gatsby-cli/src/structured-errors/error-map.ts

@@ -641,7 +641,17 @@ const errors = {
     text: ({ inputManifest, pagePath }): string =>
       `${getSharedNodeManifestWarning(
         inputManifest
-      )} but Gatsby didn't find a ownerNodeId for the page at ${pagePath}\nUsing the first page that was found with the node manifest id set in pageContext.id in createPage().\nThis may result in an inaccurate node manifest (for previews or other purposes).`,
+      )} but Gatsby didn't find an ownerNodeId for the page at ${pagePath}\nUsing the first page that was found with the node manifest id set in pageContext.id in createPage().\nThis may result in an inaccurate node manifest (for previews or other purposes).`,
+    level: Level.WARNING,
+    category: ErrorCategory.USER,
+  },
+
+  "11805": {
+    // @todo add docs link to "using Preview" once it's updated with an explanation of ownerNodeId
+    text: ({ inputManifest, pagePath }): string =>
+      `${getSharedNodeManifestWarning(
+        inputManifest
+      )} but Gatsby didn't find an ownerNodeId for the page at ${pagePath}\nUsing the first page that was found with the node manifest id set in pageContext.slug in createPage().\nThis may result in an inaccurate node manifest (for previews or other purposes).`,
     level: Level.WARNING,
     category: ErrorCategory.USER,
   },
@@ -651,7 +661,7 @@ const errors = {
     text: ({ inputManifest, pagePath }): string =>
       `${getSharedNodeManifestWarning(
         inputManifest
-      )} but Gatsby didn't find a ownerNodeId for the page at ${pagePath}\nUsing the first page where this node is queried.\nThis may result in an inaccurate node manifest (for previews or other purposes).`,
+      )} but Gatsby didn't find an ownerNodeId for the page at ${pagePath}\nUsing the first page where this node is queried.\nThis may result in an inaccurate node manifest (for previews or other purposes).`,
     level: Level.WARNING,
     category: ErrorCategory.USER,
   },

packages/gatsby-plugin-gatsby-cloud/src/utils/use-poll-for-node-manifest/types.ts

@@ -12,6 +12,7 @@ type FoundPageBy =
   | `ownerNodeId`
   | `filesystem-route-api`
   | `context.id`
+  | `context.slug`
   | `queryTracking`
   | `none`
 

packages/gatsby/src/utils/node-manifest.ts

@@ -28,11 +28,12 @@ type FoundPageBy =
   | `filesystem-route-api`
   // for these three we warn to use ownerNodeId instead
   | `context.id`
+  | `context.slug`
   | `queryTracking`
   | `none`
 
 /**
- * Finds a final built page by nodeId
+ * Finds a final built page by nodeId or by node.slug as a fallback.
  *
  * Note that this function wont work properly in `gatsby develop`
  * since develop no longer runs all page queries when creating pages.
@@ -41,7 +42,13 @@ type FoundPageBy =
  * When this fn is being used for routing to previews the user wont necessarily have
  * visited the page in the browser yet.
  */
-async function findPageOwnedByNodeId({ nodeId }: { nodeId: string }): Promise<{
+async function findPageOwnedByNode({
+  nodeId,
+  slug,
+}: {
+  nodeId: string
+  slug?: string
+}): Promise<{
   page: INodeManifestPage
   foundPageBy: FoundPageBy
 }> {
@@ -78,10 +85,10 @@ async function findPageOwnedByNodeId({ nodeId }: { nodeId: string }): Promise<{
       // if we haven't found a page with this nodeId
       // set as page.ownerNodeId then run this logic.
       // this condition is on foundOwnerNodeId instead of ownerPagePath
-      // in case we find a page with the nodeId in page.context.id
+      // in case we find a page with the nodeId in page.context.id/context.slug
       // and then later in the loop there's a page with this nodeId
       // set on page.ownerNodeId.
-      // We always want to prefer ownerPagePath over context.id
+      // We always want to prefer ownerPagePath over context.id/context.slug
       if (foundOwnerNodeId) {
         break
       }
@@ -98,7 +105,10 @@ async function findPageOwnedByNodeId({ nodeId }: { nodeId: string }): Promise<{
 
       foundOwnerNodeId = fullPage?.ownerNodeId === nodeId
 
-      const foundPageIdInContext = fullPage?.context.id === nodeId
+      const foundPageIdInContext = fullPage?.context?.id === nodeId
+
+      // querying by node.slug in GraphQL queries is common enough that we can search for it as a fallback after ownerNodeId, filesystem routes, and context.id
+      const foundPageSlugInContext = slug && fullPage?.context?.slug === slug
 
       if (foundOwnerNodeId) {
         foundPageBy = `ownerNodeId`
@@ -113,6 +123,8 @@ async function findPageOwnedByNodeId({ nodeId }: { nodeId: string }): Promise<{
         foundPageBy = pageCreatedByFilesystemPlugin
           ? `filesystem-route-api`
           : `context.id`
+      } else if (foundPageSlugInContext && fullPage) {
+        foundPageBy = `context.slug`
       }
 
       if (
@@ -128,7 +140,8 @@ async function findPageOwnedByNodeId({ nodeId }: { nodeId: string }): Promise<{
           // that's mapped to this node id.
           // this also makes this work with the filesystem Route API without
           // changing that API.
-          foundPageIdInContext)
+          foundPageIdInContext ||
+          foundPageSlugInContext)
       ) {
         // save this path to use in our manifest!
         ownerPagePath = fullPage.path
@@ -153,6 +166,7 @@ async function findPageOwnedByNodeId({ nodeId }: { nodeId: string }): Promise<{
 export const foundPageByToLogIds = {
   none: `11801`,
   [`context.id`]: `11802`,
+  [`context.slug`]: `11805`,
   queryTracking: `11803`,
   [`filesystem-route-api`]: `success`,
   ownerNodeId: `success`,
@@ -177,6 +191,7 @@ export function warnAboutNodeManifestMappingProblems({
   switch (foundPageBy) {
     case `none`:
     case `context.id`:
+    case `context.slug`:
     case `queryTracking`: {
       logId = foundPageByToLogIds[foundPageBy]
       if (verbose) {
@@ -236,8 +251,10 @@ export async function processNodeManifest(
   }
 
   // map the node to a page that was created
-  const { page: nodeManifestPage, foundPageBy } = await findPageOwnedByNodeId({
+  const { page: nodeManifestPage, foundPageBy } = await findPageOwnedByNode({
     nodeId,
+    // querying by node.slug in GraphQL queries is common enough that we can search for it as a fallback after ownerNodeId, filesystem routes, and context.id
+    slug: fullNode?.slug as string,
   })
 
   const nodeManifestMappingProblemsContext = {