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.

Sign up for GitHub

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

Bugfix : gatsby-source-wordpress giving invalid node names to GraphQL #1778

Merged
merged 5 commits into from
Aug 16, 2017
Merged

Bugfix : gatsby-source-wordpress giving invalid node names to GraphQL #1778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
743d15e
Bugfix : node undefined
sebastienfi Aug 11, 2017
676d7f5
Removed unecessary stuff from example site
sebastienfi Aug 11, 2017
c73079b
Merge branch 'master' into sebastienfi-patch-1
sebastienfi Aug 14, 2017
ef0a2ab
Merge branch 'master' into sebastienfi-patch-1
KyleAMathews Aug 15, 2017
98dbdea
Merge branch 'master' into sebastienfi-patch-1
sebastienfi Aug 15, 2017
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
37 changes: 11 additions & 26 deletions examples/using-wordpress/src/templates/post.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import React from "react"
import React, { Component } from "react"
import PropTypes from "prop-types"

import Helmet from "react-helmet"
import Header from "../components/Header"
import Footer from "../components/Footer"
import { H1, Row, Page, Column } from "../components/styled"

class PostTemplate extends React.Component {
class PostTemplate extends Component {
render() {
const post = this.props.data.wordpressPost
// console.log(`this.props is`, this.props)

const post = this.props.data.wordpressPost
const wordpressPages = this.props.data.allWordpressPage

const siteMetadata = this.props.data.site.siteMetadata

return (
Expand Down Expand Up @@ -39,6 +41,11 @@ class PostTemplate extends React.Component {
}
//<img src={post.image.sizes.thumbnail} />

PostTemplate.propTypes = {
data: PropTypes.object.isRequired,
edges: PropTypes.array,
}

export default PostTemplate

export const pageQuery = graphql`
Expand Down Expand Up @@ -111,28 +118,6 @@ export const pageQuery = graphql`
}
}
}
allWordpressTag {
edges {
node {
id
slug
description
name
taxonomy
}
}
}
allWordpressCategory {
edges {
node {
id
description
name
slug
taxonomy
}
}
}
site {
id
siteMetadata {
Expand Down
170 changes: 89 additions & 81 deletions packages/gatsby-source-wordpress/src/gatsby-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,44 +68,51 @@ exports.sourceNodes = async (
url = `${_siteURL}/wp-json`
}

console.log()
console.log(
colorized.out(
`=START PLUGIN=====================================`,
colorized.color.Font.FgBlue
if (_verbose) console.log()
Copy link
Contributor

Choose a reason for hiding this comment

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

plugins are now passed a reporter object for doing this, if you want to make use of it. e.g. reporter.verbose('blah')

Copy link
Contributor

Choose a reason for hiding this comment

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

@jquense it'd be cool to auto-prefix the plugin name as well to messages

if (_verbose)
console.log(
colorized.out(
`=START PLUGIN=====================================`,
colorized.color.Font.FgBlue
)
)
)
console.time(`=END PLUGIN=====================================`)
console.log(``)
console.log(
colorized.out(`Site URL: ${_siteURL}`, colorized.color.Font.FgBlue)
)
console.log(
colorized.out(
`Site hosted on Wordpress.com: ${hostingWPCOM}`,
colorized.color.Font.FgBlue
if (_verbose) console.time(`=END PLUGIN=====================================`)
if (_verbose) console.log(``)
if (_verbose)
console.log(
colorized.out(`Site URL: ${_siteURL}`, colorized.color.Font.FgBlue)
)
)
console.log(
colorized.out(`Using ACF: ${useACF}`, colorized.color.Font.FgBlue)
)
console.log(
colorized.out(
`Using Auth: ${_auth.htaccess_user} ${_auth.htaccess_pass}`,
colorized.color.Font.FgBlue
if (_verbose)
console.log(
colorized.out(
`Site hosted on Wordpress.com: ${hostingWPCOM}`,
colorized.color.Font.FgBlue
)
)
)
console.log(
colorized.out(
`Verbose output: ${verboseOutput}`,
colorized.color.Font.FgBlue
if (_verbose)
console.log(
colorized.out(`Using ACF: ${useACF}`, colorized.color.Font.FgBlue)
)
)
console.log(``)
console.log(
colorized.out(`Mama Route URL: ${url}`, colorized.color.Font.FgBlue)
)
console.log(``)
if (_verbose)
console.log(
colorized.out(
`Using Auth: ${_auth.htaccess_user} ${_auth.htaccess_pass}`,
colorized.color.Font.FgBlue
)
)
if (_verbose)
console.log(
colorized.out(
`Verbose output: ${verboseOutput}`,
colorized.color.Font.FgBlue
)
)
if (_verbose) console.log(``)
if (_verbose)
console.log(
colorized.out(`Mama Route URL: ${url}`, colorized.color.Font.FgBlue)
)
if (_verbose) console.log(``)

// Touch existing Wordpress nodes so Gatsby doesn`t garbage collect them.
_.values(store.getState().nodes)
Expand Down Expand Up @@ -136,18 +143,19 @@ exports.sourceNodes = async (
if (allRoutes) {
let validRoutes = getValidRoutes(allRoutes, url, baseUrl)

console.log(``)
console.log(
colorized.out(
`Fetching the JSON data from ${validRoutes.length} valid API Routes...`,
colorized.color.Font.FgBlue
if (_verbose) console.log(``)
if (_verbose)
console.log(
colorized.out(
`Fetching the JSON data from ${validRoutes.length} valid API Routes...`,
colorized.color.Font.FgBlue
)
)
)
console.log(``)
if (_verbose) console.log(``)

for (let route of validRoutes) {
await fetchData(route, createNode)
console.log(``)
if (_verbose) console.log(``)
}

for (let item of _parentChildNodes) {
Expand All @@ -163,7 +171,8 @@ exports.sourceNodes = async (
},
})

console.timeEnd(`=END PLUGIN=====================================`)
if (_verbose)
console.timeEnd(`=END PLUGIN=====================================`)
} else {
console.log(
colorized.out(`No routes to fetch. Ending.`, colorized.color.Font.FgRed)
Expand Down Expand Up @@ -280,19 +289,21 @@ async function getWPCOMAccessToken() {
*/
function httpExceptionHandler(e) {
const { status, statusText, data: { message } } = e.response
console.log(
colorized.out(
`The server response was "${status} ${statusText}"`,
colorized.color.Font.FgRed
)
)
if (message) {
if (_verbose)
console.log(
colorized.out(
`Inner exception message : "${message}"`,
`The server response was "${status} ${statusText}"`,
colorized.color.Font.FgRed
)
)
if (message) {
if (_verbose)
console.log(
colorized.out(
`Inner exception message : "${message}"`,
colorized.color.Font.FgRed
)
)
}
}

Expand Down Expand Up @@ -389,12 +400,13 @@ function getValidRoutes(allRoutes, url, baseUrl) {
)
if (_hostingWPCOM) {
// TODO : Need to test that out with ACF on Wordpress.com hosted site. Need a premium account on wp.com to install extensions.
console.log(
colorized.out(
`The ACF options pages is untested under wordpress.com hosting. Please let me know if it works.`,
colorized.color.Effect.Blink
if (_verbose)
console.log(
colorized.out(
`The ACF options pages is untested under wordpress.com hosting. Please let me know if it works.`,
colorized.color.Effect.Blink
)
)
)
}
}

Expand Down Expand Up @@ -432,18 +444,20 @@ async function fetchData(route, createNode, parentNodeId) {
const url = route.url

if (parentNodeId != undefined) {
console.log(
colorized.out(`Extended node content`, colorized.color.Font.FgBlue),
url
)
if (_verbose)
console.log(
colorized.out(`Extended node content`, colorized.color.Font.FgBlue),
url
)
} else {
console.log(
colorized.out(
`=== [ Fetching ${type} ] ===`,
colorized.color.Font.FgBlue
),
url
)
if (_verbose)
console.log(
colorized.out(
`=== [ Fetching ${type} ] ===`,
colorized.color.Font.FgBlue
),
url
)
if (_verbose) console.time(`Fetching the ${type} took`)
}

Expand All @@ -467,7 +481,10 @@ async function fetchData(route, createNode, parentNodeId) {
length = Object.keys(routeResponse).length
}
console.log(
colorized.out(`${type} fetched : ${length}`, colorized.color.Font.FgGreen)
colorized.out(
` -> ${type} fetched : ${length}`,
colorized.color.Font.FgGreen
)
)
}

Expand Down Expand Up @@ -498,9 +515,7 @@ function createGraphQLNode(ent, type, createNode, parentNodeId) {
children: [],
parent: `__SOURCE__`,
internal: {
type: type.toUpperCase(),
content: JSON.stringify(node),
mediaType: `text/html`,
type: type,
},
}

Expand Down Expand Up @@ -530,6 +545,7 @@ function createGraphQLNode(ent, type, createNode, parentNodeId) {
node.excerpt = ent.excerpt.rendered
}

node.internal.content = JSON.stringify(node)
node.internal.contentDigest = digest(stringify(node))
createNode(node)

Expand Down Expand Up @@ -559,7 +575,6 @@ function addFields(ent, newEnt, createNode) {
internal: {
type: `${typePrefix}ACF_Field`,
content: JSON.stringify(ent.acf),
mediaType: `application/json`,
},
}
acfNode.internal.contentDigest = digest(stringify(acfNode))
Expand Down Expand Up @@ -616,8 +631,8 @@ function recursiveAddFields(ent, newEnt) {
function getValidName(key) {
let nkey = key
const NAME_RX = /^[_a-zA-Z][_a-zA-Z0-9]*$/
if (!NAME_RX.test(nkey)) {
nkey = `_${nkey}`.replace(/-/g, `_`).replace(/:/g, `_`)
if (!NAME_RX.test(nkey) || restrictedNodeFields.includes(nkey)) {
nkey = `${conflictFieldPrefix}${nkey}`.replace(/-|__|:|\.|\s/g, `_`)
Copy link
Contributor

@jquense jquense Aug 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

so we should provide a single robust utility for doing this (there is one in gatsby), but this is still incomplete in terms of what graphql will allow. It needs to replace any character that isn't alphanumeric or a _. It also can't start with a number or two or more _. e.g here are some fields that would still error out here.

  • "##foo"
  • "1number"

if (_verbose)
console.log(
colorized.out(
Expand All @@ -626,12 +641,5 @@ function getValidName(key) {
)
)
}
if (restrictedNodeFields.includes(nkey)) {
if (_verbose)
console.log(
`Restricted field found for ${nkey}. Prefixing with ${conflictFieldPrefix}.`
)
nkey = `${conflictFieldPrefix}${nkey}`
}
return nkey
}