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

RFC: File System Routing Improvements #24463

Merged
merged 4 commits into from
Aug 3, 2020
Merged

Conversation

blainekasten
Copy link
Contributor

@blainekasten blainekasten commented May 26, 2020

@blainekasten blainekasten requested a review from a team as a code owner May 26, 2020 02:15
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 26, 2020
@blainekasten blainekasten added status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged type: rfc and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 26, 2020
@blainekasten blainekasten changed the title RFC: Secret RFC: File System Routing Improvements Jun 18, 2020
@blainekasten blainekasten removed the status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged label Jun 18, 2020
@herecydev
Copy link
Contributor

@blainekasten looks really good, I've got an example that I currently use on a Gatsby site ~10K pages. I'll try to create a relevant scenario with the products collection builder.

So you've used /src/pages/product/{id}.js to create /product/1 (burger) etc. What if you wanted to nest some paths such that you can dynamically create /product/1/reviews/1, /product/1/reviews/2, etc.

Currently this is trivial as you just need:

//gatsby-node.js

products.forEach(product => {
//create product page
createPage()

product.reviews.forEach(review => {
//create review page
createPage()
}
})

I wonder if the createPagesFromData could take an array such that the following works:

// filepath:
// /src/pages/product/{id}/reviews/{name}.js
//
// urls:
// /product/1/reviews/newyorktimes (burger)
// /product/2/reviews/chickenlovers (chicken)
//

// This macro fetches the Product's, and generates a page for each one with the component given.
export default createPagesFromData([[Product, `Product`], [Review, `Review`]])

// This component then can fetch it's own data to use inside of the component.
export const query = graphql`
  query RootProduct($id: String) {
    product(id: { eq: $id }) {
      id
      name
    }
    review(id: { eq: $id }) {
      id
      name
    }
  }
`

Each "braced" template will just take the matching item of the array. Thoughts?

@blainekasten
Copy link
Contributor Author

@herecydev Thanks for the feedback on this! So our goal with this API is to solve the 90% of use-cases. Working with multiple data sources is a bit tricky. Is the product and reviewed tied together in the node module? If it is you could do something like this:

/src/pages/product/{id}/reviews/{reviews__name}.js

This is assuming there is some structure like this:

query Product {
  id
  reviews {
    name
  }
}

If this doesn't exist already, you could make the connection in gatsby-node, or even create a new data structure that marries the two types.

I'm curious how frequently people are merging two entirely unique data types together to decide if that warrants specific API support.

@herecydev
Copy link
Contributor

So our goal with this API is to solve the 90% of use-cases

Yer I get that, hard to quantify how big a problem it will be.

If this doesn't exist already, you could make the connection in gatsby-node, or even create a new data structure that marries the two types.

That's awesome, I assume any "magic" like @links or schemaCustomizations should work as well. Basically as long as you could query that normally...

@advancingu
Copy link

How would the approach proposed here look like to create individual pages for localized products while additionally requiring query pagination?

As an example, let's consider the following GraphQL query for products stored in commercetools:

query MyProductsQuery($locale: commercetools_Locale, $offset: Int) {
  ct {
    products(offset: $offset) {
      results {
        masterData {
          current {
            slug(locale: $locale)
          }
        }
      }
    }
  }
}

We use $locale to get a product representation for each desired locale and $offset to paginate through all products. Note that in the case of commercetools, there is a hard query limit of 500 results enforced on the API. As for $locale, for the purposes of this example, we can consider the values available as an array hardcoded locally.

As an alternative, we could retrieve all available locales as an array of locale, value tuples (but would then need to discard those locales we are not interested in using):

query MyProductsQuery($offset: Int) {
  ct {
    products(offset: $offset) {
      results {
        masterData {
          current {
            slugAllLocales {
              locale
              value
            }
          }
        }
      }
    }
  }
}

The goal in both cases is to create product pages located at /{locale}/product/{slug}.

@gatsby-cloud
Copy link

gatsby-cloud bot commented Aug 3, 2020

Gatsby Cloud Build Report

client-only-paths

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 12s

Performance

Lighthouse report

Metric Score
Performance 💚 100
Accessibility 🔶 85
Best Practices 💚 100
SEO 🔶 70

🔗 View full report

@gatsby-cloud
Copy link

gatsby-cloud bot commented Aug 3, 2020

Gatsby Cloud Build Report

using-styled-components

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 13s

Performance

Lighthouse report

Metric Score
Performance 💚 100
Accessibility 💚 90
Best Practices 💚 100
SEO 💚 90

🔗 View full report

@gatsby-cloud-staging
Copy link

gatsby-cloud-staging bot commented Aug 3, 2020

Gatsby Cloud Build Report

gatsby-master

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 14s

Performance

Lighthouse report

Metric Score
Performance 💚 95
Accessibility 🔶 87
Best Practices 💚 93
SEO 🔶 73

🔗 View full report

@gatsby-cloud
Copy link

gatsby-cloud bot commented Aug 3, 2020

Gatsby Cloud Build Report

🚩 Your build failed. See the build logs here

Errors

Error getting repo data for starter "gatsby-starter-styled":

            Unexpected token : in JSON at position 3 in "https://raw.githubusercontent.com/gregoralbrecht/gatsby-starter-styled/master/package.json": 
404: Not Found...

@gatsby-cloud
Copy link

gatsby-cloud bot commented Aug 3, 2020

Gatsby Cloud Build Report

🚩 Your build failed. See the build logs here

Errors

Error getting repo data for starter "gatsby-starter-styled":

            Unexpected token : in JSON at position 3 in "https://raw.githubusercontent.com/gregoralbrecht/gatsby-starter-styled/master/package.json": 
404: Not Found...

@blainekasten blainekasten force-pushed the rfc/gatsby-days-release branch 2 times, most recently from a8f13a2 to 8688fe2 Compare August 3, 2020 20:03
@blainekasten blainekasten force-pushed the rfc/gatsby-days-release branch from 8688fe2 to 27cfd35 Compare August 3, 2020 20:04
@blainekasten blainekasten merged commit dd6550c into master Aug 3, 2020
@delete-merged-branch delete-merged-branch bot deleted the rfc/gatsby-days-release branch August 3, 2020 20:25
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

Successfully merging this pull request may close these issues.

3 participants