Skip to content
Closed
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
38 changes: 33 additions & 5 deletions packages/gatsby-source-contentful/src/fetch.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,35 @@
const contentful = require(`contentful`)
const _ = require(`lodash`)
const fs = require(`fs-extra`)

const normalize = require(`./normalize`)

module.exports = async ({ spaceId, accessToken, host, syncToken }) => {
const cacheFilename = `contentful-result.json`

async function writeCacheFile(dir, result) {
try {
await fs.writeJson(`${dir}/${cacheFilename}`, result)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use path.join.

} catch (err) {
console.error(err)
}
}

module.exports = async ({ spaceId, accessToken, host, syncToken, cacheDir }) => {
// Fetch articles.
console.time(`Fetch Contentful data`)

// If the user knows they are offline, serve them cached result
if (process.env.GATSBY_CONTENTFUL_OFFLINE === `true`) {
console.log(`Using Contentful Offline cache ⚠️`)
try {
return await fs.readJson(`${cacheDir}/${cacheFilename}`)
}
catch (err) {
console.error(`Sorry, cached CMS data does not exist - unable to run offline`)
process.exit(1)
}
}

console.log(`Starting to fetch data from Contentful`)

const client = contentful.createClient({
Expand All @@ -29,9 +53,10 @@ module.exports = async ({ spaceId, accessToken, host, syncToken }) => {
console.log(
`Accessing your Contentful space failed. Perhaps you're offline or the spaceId/accessToken is incorrect.`
)
// TODO perhaps continue if there's cached data? That would let
// someone develop a contentful site even if not connected to the internet.
// For prod builds though always fail if we can't get the latest data.
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure what the optimal user workflow is here. I've opted for forcing the user to acknowledge a loss of connectivity rather than trying to auto serve cache if network is down.

Copy link
Contributor

Choose a reason for hiding this comment

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

Opt-in require user to know about this feature in first place and if they don't have connectivity they can't actually look for solution for this online.

There are 2 ways to solve it:

  • do automatic fallback and add console message that cached data is used because plugin couldn't connect with contentful api
  • log that plugin can't connect and provide information how to enable this feature

Copy link
Contributor

@pieh pieh Feb 11, 2018

Choose a reason for hiding this comment

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

My bad, responded to your comment without reading your code change. I think this is better than other solution as this will cause less confusion that possibly stale data is used and requiring user action means user will acknowledge use of cached data.

Copy link
Author

Choose a reason for hiding this comment

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

cool 👍

console.log(
`Try running setting GATSBY_CONTENTFUL_OFFLINE=true to see if we can serve from cache.`
)

process.exit(1)
}

Expand Down Expand Up @@ -84,12 +109,15 @@ module.exports = async ({ spaceId, accessToken, host, syncToken }) => {
return null
})

return {
const result = {
currentSyncData,
contentTypeItems,
defaultLocale,
locales: space.locales,
}

writeCacheFile(cacheDir, result)
return result
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby-source-contentful/src/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ exports.sourceNodes = async (
spaceId,
accessToken,
host,
cacheDir: `${store.getState().program.directory}/.cache`,
Copy link
Contributor

Choose a reason for hiding this comment

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

As should this

})

const entryList = normalize.buildEntryList({
Expand Down