[source-contentful] offline mode #5869

Khaledgarbaya · 2018-06-13T09:52:16Z

This PR adds an option to the plugin fallbackToOffline that allows the user to continue working on their website if they don't have internet connection

Khaledgarbaya · 2018-06-13T09:53:07Z

@pieh here is the continuation of #3977

gatsbybot · 2018-06-13T09:58:22Z

Deploy preview for using-drupal ready!

Built with commit 4bde403

https://deploy-preview-5869--using-drupal.netlify.com

pieh · 2018-06-13T09:58:57Z

very first impression - change from env var to config might be problematic here - people would need to set it in advance - because when they would change gatsby-config.js when they are already offline it would cause .cache directory to be deleted

gatsbybot · 2018-06-13T10:01:13Z

Deploy preview for gatsbygram ready!

Built with commit 4bde403

https://deploy-preview-5869--gatsbygram.netlify.com

Khaledgarbaya · 2018-06-13T11:01:13Z

Ooh, that makes sense! I will revert that change

pieh · 2018-06-13T11:48:47Z

packages/gatsby-source-contentful/src/fetch.js

+  // If the user knows they are offline, serve them cached result
+  if (process.env.GATSBY_CONTENTFUL_OFFLINE === `true`) {
+    console.log(`Using Contentful Offline cache ⚠️`)
+    console.log(`Cache may be invalidated if you edit package.json, gatsby-node.js or gatsby-config.js files`)


Maybe those 2 lines could be moved after we read cached data, so it would would print out only if we actually can use it? Not totally sure on this, but this might be better develop experience?

pieh · 2018-06-13T11:50:25Z

packages/gatsby-source-contentful/src/fetch.js

  // 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`) {


I think we should address very valid comment that is being removed below:

// For prod builds though always fail if we can't get the latest data

and add check here to only try read cache if we are running gatsby develop. What do You think?

Sound good to me how do we know that gatsby develop was used is there a command line var set?

pieh · 2018-06-13T11:52:40Z

packages/gatsby-source-contentful/src/gatsby-node.js

@@ -65,8 +65,8 @@ exports.sourceNodes = async (
    syncToken,
    spaceId,
    accessToken,
-    environment,


Is this removal intentional? I think fetchData does need environment param to work correctly

Khaledgarbaya · 2018-06-13T12:07:35Z

One more thing I think I will change the PR a bit so the offline fallback kicks in only when the server is not reachable, I can use is-reachable library

KyleAMathews · 2018-06-13T12:28:37Z

Any reason you're not using touchNode? Gatsby garbage collects nodes that aren't created again but if a source plugin touches a node, it won't be GCed. See gatsby-source-drupal for an example of a plugin that uses this.

pieh · 2018-06-13T12:30:11Z

packages/gatsby-source-contentful/src/fetch.js

@@ -19,7 +19,8 @@ module.exports = async ({ spaceId, accessToken, host, syncToken, cacheDir, envir
  console.time(`Fetch Contentful data`)

  // If the user knows they are offline, serve them cached result
-  if (process.env.GATSBY_CONTENTFUL_OFFLINE === `true`) {
+  // For prod builds though always fail if we can't get the latest data
+  if (process.env.GATSBY_CONTENTFUL_OFFLINE === `true` && process.env.ACTIVE_ENV === `development`) {


maybe

- if (process.env.GATSBY_CONTENTFUL_OFFLINE === `true` && process.env.ACTIVE_ENV === `development`) { + if (process.env.GATSBY_CONTENTFUL_OFFLINE === `true` && process.env.NODE_ENV !== `production`) {

?
But yeah, we should probably make it easier to determine if we are in build or develop

Khaledgarbaya · 2018-06-13T12:54:17Z

@KyleAMathews oh I was not aware of that, do you mean instead of saving the data to a json file we just touch the nodes?

KyleAMathews · 2018-06-13T12:55:31Z

Yeah, gatsby already saves the cache. Instead you just grab all nodes created by this plugin and touch them.

uttrasey · 2018-06-13T13:29:59Z

Thanks for running with this @Khaledgarbaya 👍

Khaledgarbaya · 2018-06-13T13:35:53Z

ok @pieh @KyleAMathews I made the change to use touchNode instead I hope I didn't miss anything

pieh · 2018-06-13T14:13:32Z

packages/gatsby-source-contentful/src/gatsby-node.js

+    process.env.ACTIVE_ENV !== `production`
+  ) {
+
+    _.values(store.getState().nodes)


You can use

-_.values(store.getState().nodes) +getNodes()

here

pieh · 2018-06-13T14:13:56Z

packages/gatsby-source-contentful/src/gatsby-node.js

+  ) {
+
+    _.values(store.getState().nodes)
+      .filter(n => n.internal.type === `gatsby-source-contentful`)


I think You need owner. not type

pieh · 2018-06-13T14:14:45Z

packages/gatsby-source-contentful/src/gatsby-node.js

    host,
+    environment,
+    cacheDir: `${store.getState().program.directory}/.cache`,


cacheDir not needed anymore I think?

Khaledgarbaya · 2018-06-15T07:25:48Z

@pieh I think this is ready for review

…ffline

pieh

working as expected, very nice!

pieh

Re-approving and merging! Sorry you had to resolve package.json conflicts because I forgot to merge this in!

* Add fallback to offline * Fix lint * Cleanup * small fixes * format

pieh mentioned this pull request Jun 13, 2018

Allow Gatsby/Contentful to work offline #3977

Closed

pieh reviewed Jun 13, 2018

View reviewed changes

Add fallback to offline

4795c19

Fix lint

9e12526

pieh reviewed Jun 13, 2018

View reviewed changes

Cleanup

ee28556

Khaledgarbaya and others added 4 commits June 19, 2018 10:53

Merge branch 'master' into feat/contentful-offline

21eefe6

Merge remote-tracking branch 'upstream/master' into feat/contentful-o…

e48e156

…ffline

small fixes

09565b9

format

c4d7219

pieh previously approved these changes Jun 19, 2018

View reviewed changes

Merge branch 'master' into feat/contentful-offline

4bde403

Khaledgarbaya dismissed pieh’s stale review via 4bde403 June 21, 2018 11:27

pieh approved these changes Jun 21, 2018

View reviewed changes

pieh merged commit bd5d76e into gatsbyjs:master Jun 21, 2018

Khaledgarbaya deleted the feat/contentful-offline branch June 21, 2018 13:27

KyleAMathews pushed a commit that referenced this pull request Jun 21, 2018

[source-contentful] offline mode (#5869)

5ac9714

* Add fallback to offline * Fix lint * Cleanup * small fixes * format

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[source-contentful] offline mode #5869

[source-contentful] offline mode #5869

Khaledgarbaya commented Jun 13, 2018

Khaledgarbaya commented Jun 13, 2018

gatsbybot commented Jun 13, 2018 •

edited

Loading

pieh commented Jun 13, 2018

gatsbybot commented Jun 13, 2018 •

edited

Loading

Khaledgarbaya commented Jun 13, 2018

pieh Jun 13, 2018

pieh Jun 13, 2018 •

edited

Loading

Khaledgarbaya Jun 13, 2018

pieh Jun 13, 2018

Khaledgarbaya Jun 13, 2018

Khaledgarbaya commented Jun 13, 2018

KyleAMathews commented Jun 13, 2018

pieh Jun 13, 2018

Khaledgarbaya commented Jun 13, 2018

KyleAMathews commented Jun 13, 2018

uttrasey commented Jun 13, 2018

Khaledgarbaya commented Jun 13, 2018

pieh Jun 13, 2018

pieh Jun 13, 2018

pieh Jun 13, 2018

Khaledgarbaya commented Jun 15, 2018

pieh left a comment

pieh left a comment

[source-contentful] offline mode #5869

[source-contentful] offline mode #5869

Conversation

Khaledgarbaya commented Jun 13, 2018

Khaledgarbaya commented Jun 13, 2018

gatsbybot commented Jun 13, 2018 • edited Loading

pieh commented Jun 13, 2018

gatsbybot commented Jun 13, 2018 • edited Loading

Khaledgarbaya commented Jun 13, 2018

Choose a reason for hiding this comment

pieh Jun 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Khaledgarbaya commented Jun 13, 2018

KyleAMathews commented Jun 13, 2018

Choose a reason for hiding this comment

Khaledgarbaya commented Jun 13, 2018

KyleAMathews commented Jun 13, 2018

uttrasey commented Jun 13, 2018

Khaledgarbaya commented Jun 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Khaledgarbaya commented Jun 15, 2018

pieh left a comment

Choose a reason for hiding this comment

pieh left a comment

Choose a reason for hiding this comment

gatsbybot commented Jun 13, 2018 •

edited

Loading

gatsbybot commented Jun 13, 2018 •

edited

Loading

pieh Jun 13, 2018 •

edited

Loading