chore(www): Factor out prev-and-next calculation to its own util #20978
Conversation
| ) | ||
| } | ||
|
|
||
| function getPrevAndNext(slug) { |
There was a problem hiding this comment.
everything is copy-pasta'd, except I renamed it from nextAndPrev to prevAndNext to make it line up with the prev-and-next component (and because it makes more sense).
|
|
||
| // add values to page context for next and prev page | ||
| let prevAndNext = {} | ||
| if (docIndex > -1) { |
There was a problem hiding this comment.
could probably do some refactoring here but I'm too lazy and don't wanna break things.
There was a problem hiding this comment.
We should be doing things intentionally, can you expand more on the trade-offs you're seeing here?
There was a problem hiding this comment.
From a code clarity perspective, it was hard to wrap my head around what this function is doing: "find the previous and next article in the overall list this article belongs in." We can make it more DRY (although that's a loaded term these days):
// Find the list that this slug belongs in
const list = [flattenedDocs, flattenedTutorials, flattenedContributing].find(list => list.findIndex(findDoc, { link: slug }) > -1)
const index = list.findIndex(findDoc, { link: slug })
// get the previous and next article in that list
const prevAndNext = {
prev: getSibling(index, list, 'prev')
next: getSibling(index, list, 'next')
}
As I said, it's not a big deal and I'm okay leaving the code as is. It's just my refactor brain going "wait I can make this cleaner!"
| @@ -465,7 +369,7 @@ exports.createPages = ({ graphql, actions, reporter }) => { | |||
| context: { | |||
| slug: node.fields.slug, | |||
| locale, | |||
| ...nextAndPrev, | |||
| ...getPrevAndNext(node.fields.slug), | |||
There was a problem hiding this comment.
Now that I think about it, what is the advantage of passing in this data in context instead of passing it the slug to the <PrevAndNext> component and calling getPrevAndNext from there? I don't think it relies on anything needed in gatsby-node.
There was a problem hiding this comment.
That seems like something for a followup PR, probably? Keep this scoped to moving the util?
There was a problem hiding this comment.
Yeah, that sounds good.
wardpeet
changed the title
sidharthachatterjee
added
the
bot: merge on green
Description
Move calculation of previous and next articles to its own utility file.
This annoyed me enough that I finally decided to do it. The logic is fairly complicated and it cluttered up
gatsby-node.Related Issues and PRs
This can be closed if #21064 gets in first.