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

fix(starters): Improve null checking & update to best practices #26918

Merged
merged 10 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 76 additions & 26 deletions starters/blog/gatsby-node.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
const path = require(`path`)
const { createFilePath } = require(`gatsby-source-filesystem`)

exports.createPages = async ({ graphql, actions }) => {
exports.createPages = async ({ graphql, actions, reporter }) => {
const { createPage } = actions

// Define a template for blog post
const blogPost = path.resolve(`./src/templates/blog-post.js`)

// Get all markdown blog posts sorted by date
const result = await graphql(
`
{
allMarkdownRemark(
sort: { fields: [frontmatter___date], order: DESC }
limit: 1000
) {
edges {
node {
fields {
slug
}
frontmatter {
title
}
nodes {
fields {
slug
}
frontmatter {
title
}
}
}
Expand All @@ -28,37 +29,86 @@ exports.createPages = async ({ graphql, actions }) => {
)

if (result.errors) {
throw result.errors
reporter.panicOnBuild(`There was an error loading your blog posts`, result.errors)
return
}

// Create blog posts pages.
const posts = result.data.allMarkdownRemark.edges

posts.forEach((post, index) => {
const previous = index === posts.length - 1 ? null : posts[index + 1].node
const next = index === 0 ? null : posts[index - 1].node

createPage({
path: post.node.fields.slug,
component: blogPost,
context: {
slug: post.node.fields.slug,
previous,
next,
},
const posts = result.data.allMarkdownRemark.nodes

// Create blog posts pages
// But only if there's at least one markdown file found at "content/blog" (defined in gatsby-config.js)
// `context` is available in the template as a prop and as a variable in GraphQL

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check shouldn't be needed, because if the array is empty, the forEach never happens anyway. However, what might be needed is a null check on allMarkdownRemark. Does that exist if there's no markdown? Shame there's no optional chaining in gatsby-node!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to make it more explicit as I myself can't always remember that e.g. forEach would behave this way

if (posts.length > 0) {
posts.forEach((post, index) => {
const previous = index === posts.length - 1 ? null : posts[index + 1]
const next = index === 0 ? null : posts[index - 1]

createPage({
path: post.fields.slug,
component: blogPost,
context: {
slug: post.fields.slug,
previous,
next,
},
})
})
})
}
}

exports.onCreateNode = ({ node, actions, getNode }) => {
const { createNodeField } = actions

if (node.internal.type === `MarkdownRemark`) {
const value = createFilePath({ node, getNode })

createNodeField({
name: `slug`,
node,
value,
})
}
}

exports.createSchemaCustomization = ({ actions }) => {
const { createTypes } = actions

// Explicitly define the siteMetadata {} object
// This way those will always be defined even if removed from gatsby-config.js

// Also explicitly define the Markdown frontmatter
// This way the "MarkdownRemark" queries will return `null` even when no
// blog posts are stored inside "content/blog" instead of returning an error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, well that answers my question. Does it need a null check or does it have an empty nodes array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an empty array then :)

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The markdownRemark query returns null

createTypes(`
type SiteSiteMetadata {
author: Author
siteUrl: String
social: Social
}

type Author {
name: String
summary: String
}

type Social {
twitter: String
}

type MarkdownRemark implements Node {
frontmatter: Frontmatter
fields: Fields
}

type Frontmatter {
title: String
description: String
date: Date @dateformat
}

type Fields {
slug: String
}
`)
}
51 changes: 30 additions & 21 deletions starters/blog/src/components/bio.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,34 +35,43 @@ const Bio = () => {
}
`)

const { author, social } = data.site.siteMetadata
// Set these values by editing "siteMetadata" in gatsby-config.js
const author = data.site.siteMetadata?.author
const social = data.site.siteMetadata?.social

const avatar = data?.avatar?.childImageSharp?.fixed

return (
<div
style={{
display: `flex`,
marginBottom: rhythm(2.5),
}}
>
<Image
fixed={data.avatar.childImageSharp.fixed}
alt={author.name}
style={{
marginRight: rhythm(1 / 2),
marginBottom: 0,
minWidth: 50,
borderRadius: `100%`,
}}
imgStyle={{
borderRadius: `50%`,
}}
/>
<p>
Written by <strong>{author.name}</strong> {author.summary}
{` `}
<a href={`https://twitter.com/${social.twitter}`}>
You should follow him on Twitter
</a>
</p>
{avatar && (
<Image
fixed={avatar}
alt={author?.name || ``}
style={{
marginRight: rhythm(1 / 2),
marginBottom: 0,
minWidth: 50,
borderRadius: `100%`,
}}
imgStyle={{
borderRadius: `50%`,
}}
/>
)}
{author?.name && (
<p>
Written by <strong>{author.name}</strong> {author?.summary || null}
{` `}
<a href={`https://twitter.com/${social?.twitter || ``}`}>
You should follow them on Twitter
Comment on lines +70 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think, if there is no twitter name is set, then this a tag shot not printed?

</a>
</p>
)}
</div>
)
}
Expand Down
2 changes: 1 addition & 1 deletion starters/blog/src/components/layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const Layout = ({ location, title, children }) => {
<footer>
© {new Date().getFullYear()}, Built with
{` `}
<a href="https://www.gatsbyjs.org">Gatsby</a>
<a href="https://www.gatsbyjs.com">Gatsby</a>
</footer>
</div>
)
Expand Down
5 changes: 3 additions & 2 deletions starters/blog/src/components/seo.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ const SEO = ({ description, lang, meta, title }) => {
)

const metaDescription = description || site.siteMetadata.description
const defaultTitle = site.siteMetadata?.title

return (
<Helmet
htmlAttributes={{
lang,
}}
title={title}
titleTemplate={`%s | ${site.siteMetadata.title}`}
titleTemplate={defaultTitle ? `%s | ${defaultTitle}` : null}
meta={[
{
name: `description`,
Expand All @@ -59,7 +60,7 @@ const SEO = ({ description, lang, meta, title }) => {
},
{
name: `twitter:creator`,
content: site.siteMetadata.social.twitter,
content: site.siteMetadata?.social?.twitter || ``,
},
{
name: `twitter:title`,
Expand Down
2 changes: 1 addition & 1 deletion starters/blog/src/pages/404.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const NotFoundPage = ({ data, location }) => {
return (
<Layout location={location} title={siteTitle}>
<SEO title="404: Not Found" />
<h1>Not Found</h1>
<h1>404: Not Found</h1>
<p>You just hit a route that doesn&#39;t exist... the sadness.</p>
</Layout>
)
Expand Down
46 changes: 27 additions & 19 deletions starters/blog/src/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,28 @@ import SEO from "../components/seo"
import { rhythm } from "../utils/typography"

const BlogIndex = ({ data, location }) => {
const siteTitle = data.site.siteMetadata.title
const posts = data.allMarkdownRemark.edges
const siteTitle = data.site.siteMetadata?.title || `Title`
const posts = data.allMarkdownRemark.nodes

if (posts.length === 0) {
return (
<Layout location={location} title={siteTitle}>
<SEO title="All posts" />
<Bio />
<p>No blog posts found. Add markdown posts to "content/blog" (or the directory you specified for the "gatsby-source-filesystem" plugin in gatsby-config.js).</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big fan of this approach. Nice work.

</Layout>
)
}

return (
<Layout location={location} title={siteTitle}>
<SEO title="All posts" />
<Bio />
{posts.map(({ node }) => {
const title = node.frontmatter.title || node.fields.slug
{posts.map((post) => {
const title = post.frontmatter.title || post.fields.slug
return (
<article
key={node.fields.slug}
key={post.fields.slug}
itemScope
itemType="http://schema.org/Article"
>
Expand All @@ -30,18 +40,18 @@ const BlogIndex = ({ data, location }) => {
>
<Link
style={{ boxShadow: `none` }}
to={node.fields.slug}
to={post.fields.slug}
itemProp="url"
>
<span itemProp="headline">{title}</span>
</Link>
</h3>
<small>{node.frontmatter.date}</small>
<small>{post.frontmatter.date}</small>
</header>
<section>
<p
dangerouslySetInnerHTML={{
__html: node.frontmatter.description || node.excerpt,
__html: post.frontmatter.description || post.excerpt,
}}
itemProp="description"
/>
Expand All @@ -63,17 +73,15 @@ export const pageQuery = graphql`
}
}
allMarkdownRemark(sort: { fields: [frontmatter___date], order: DESC }) {
edges {
node {
excerpt
fields {
slug
}
frontmatter {
date(formatString: "MMMM DD, YYYY")
title
description
}
nodes {
excerpt
fields {
slug
}
frontmatter {
date(formatString: "MMMM DD, YYYY")
title
description
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion starters/blog/src/pages/using-typescript.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const UsingTypescript: React.FC<PageProps<DataProps>> = ({ data, path, location
<p>This means that you can create and write <em>.ts/.tsx</em> files for your pages, components etc. Please note that the <em>gatsby-*.js</em> files (like gatsby-node.js) currently don't support TypeScript yet.</p>
<p>For type checking you'll want to install <em>typescript</em> via npm and run <em>tsc --init</em> to create a <em>.tsconfig</em> file.</p>
<p>You're currently on the page "{path}" which was built on {data.site.buildTime}.</p>
<p>To learn more, head over to our <a href="https://www.gatsbyjs.org/docs/typescript/">documentation about TypeScript</a>.</p>
<p>To learn more, head over to our <a href="https://www.gatsbyjs.com/docs/typescript/">documentation about TypeScript</a>.</p>
<Link to="/">Go back to the homepage</Link>
</Layout>
)
Expand Down
2 changes: 1 addition & 1 deletion starters/blog/src/templates/blog-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { rhythm, scale } from "../utils/typography"

const BlogPostTemplate = ({ data, pageContext, location }) => {
const post = data.markdownRemark
const siteTitle = data.site.siteMetadata.title
const siteTitle = data.site.siteMetadata?.title || `Title`
const { previous, next } = pageContext

return (
Expand Down
4 changes: 4 additions & 0 deletions starters/default/src/components/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ const Image = () => {
}
`)

if (!data?.placeholderImage?.childImageSharp?.fluid) {
LekoArts marked this conversation as resolved.
Show resolved Hide resolved
return <div>Picture not found</div>
}

return <Img fluid={data.placeholderImage.childImageSharp.fluid} />
}

Expand Down
8 changes: 5 additions & 3 deletions starters/default/src/components/layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const Layout = ({ children }) => {

return (
<>
<Header siteTitle={data.site.siteMetadata.title} />
<Header siteTitle={data.site.siteMetadata?.title || `Title`} />
<div
style={{
margin: `0 auto`,
Expand All @@ -34,10 +34,12 @@ const Layout = ({ children }) => {
}}
>
<main>{children}</main>
<footer>
<footer style={{
marginTop: `2rem`
}}>
© {new Date().getFullYear()}, Built with
{` `}
<a href="https://www.gatsbyjs.org">Gatsby</a>
<a href="https://www.gatsbyjs.com">Gatsby</a>
</footer>
</div>
</>
Expand Down
Loading