feat(gatsby): Add preliminary fast-refresh integration (#26664)

* chore(gatsby): Use fast refresh if the version of react installed supports it * Update detect-hot-loader-to-use.ts * idk * rm * fix ts * try to fix 2 tests.. * things * fixes * another thing * Fix most tests * fix the tests by moving the page query into hook until i can fix the multiple exports problem * Finally got FastRefresh working with page components * fix tests * one versino of react * test something * fix test * revert script changes * reset react version for gatsby-cli * Update packages/gatsby/src/utils/get-react-hot-loader-strategy.ts Co-authored-by: Ward Peeters <[email protected]> * updates * named exports * circleci: create new test with fast-refresh * another position lel * update tests * reset package json * package.json * lock file * wip stuff from blaine * wip stuff from blaine * re-add pieh e2e tests * consolidate into less components and react class * lunch break -- use portal, runtime error works * use components * error boundary * runtime mostly working * lint * get babel-code-frame for runtime working * lint * fix errors from merge * Revert "re-add pieh e2e tests" This reverts commit 66c6400 * only use eval for fast-refresh * more proper TS types * re-add isomporphic-fetch * only conditionally use fast-refresh overlay + safe guards for sourceMap * style * test run for CI * cypress conditionally * comment out the default for 17 thing * fix linting * fix circleci after wrong merge conflict resolution Co-authored-by: Ward Peeters <[email protected]> Co-authored-by: LekoArts <[email protected]> Co-authored-by: gatsbybot <[email protected]> Co-authored-by: Michal Piechowiak <[email protected]>
gatsbyjs · Nov 26, 2020 · 613f5c7 · 613f5c7
1 parent 2e3ec89
commit 613f5c7
Show file tree

Hide file tree

Showing 43 changed files with 920 additions and 92 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -329,6 +329,12 @@ jobs:
   e2e_tests_development_runtime:
     <<: *e2e_tests_development_runtime_alias
 
+  e2e_tests_development_runtime_fast_refresh:
+    <<: *e2e_tests_development_runtime_alias
+    environment:
+      GATSBY_HOT_LOADER: fast-refresh
+      CYPRESS_HOT_LOADER: fast-refresh
+
   e2e_tests_development_runtime_with_experimental_react:
     <<: *e2e_tests_development_runtime_alias
 
@@ -611,6 +617,8 @@ workflows:
           <<: *e2e-test-workflow
       - e2e_tests_development_runtime:
           <<: *e2e-test-workflow
+      - e2e_tests_development_runtime_fast_refresh:
+          <<: *e2e-test-workflow
       - e2e_tests_production_runtime:
           <<: *e2e-test-workflow
       - themes_e2e_tests_production_runtime:

diff --git a/.eslintrc.js b/.eslintrc.js
@@ -45,6 +45,12 @@ module.exports = {
         allowTaggedTemplates: true,
       },
     ],
+    "no-unused-vars": [
+      "warn",
+      {
+        varsIgnorePattern: "^_"
+      }
+    ],
     "consistent-return": ["error"],
     "filenames/match-regex": ["error", "^[a-z-\\d\\.]+$", true],
     "no-console": "off",

diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js b/e2e-tests/development-runtime/cypress/integration/functionality/query-data-caches.js
@@ -87,18 +87,15 @@ function pageTitleAndDataAssertion(config) {
     cy.findByText(`Preview custom 404 page`).click()
   }
 
-  cy.getTestElement(`${config.prefix || ``}page-path`)
-    .invoke(`text`)
-    .should(`equal`, getExpectedCanonicalPath(config))
+  cy.findByTestId(`${config.prefix || ``}page-path`)
+    .should(`have.text`, getExpectedCanonicalPath(config))
 
-  cy.getTestElement(`${config.prefix || ``}query-data-caches-page-title`)
-    .invoke(`text`)
-    .should(`equal`, `This is page ${config.page}`)
+  cy.findByTestId(`${config.prefix || ``}query-data-caches-page-title`)
+    .should(`have.text`, `This is page ${config.page}`)
 
-  cy.getTestElement(`${config.prefix || ``}${config.queryType}-query-result`)
-    .invoke(`text`)
+  cy.findByTestId(`${config.prefix || ``}${config.queryType}-query-result`)
     .should(
-      `equal`,
+      `have.text`,
       `${config.slug} / ${
         config.page === config.initialPage ? `initial-page` : `second-page`
       }: ${config.data}`
@@ -119,7 +116,7 @@ function runTests(config) {
     data: `before-edit`,
   })
 
-  cy.getTestElement(`page-b-link`).click().waitForRouteChange()
+  cy.findByTestId(`page-b-link`).click().waitForRouteChange()
 
   // assert we navigated
   pageTitleAndDataAssertion({ ...config, page: `B`, data: `before-edit` })
@@ -131,10 +128,10 @@ function runTests(config) {
   pageTitleAndDataAssertion({ ...config, page: `B`, data: `after-edit` })
 
   if (config.navigateBack === `link`) {
-    cy.getTestElement(`page-a-link`).click().waitForRouteChange()
+    cy.findByTestId(`page-a-link`).click().waitForRouteChange()
   } else if (config.navigateBack === `history`) {
     // this is just making sure page components don't have link to navigate back (asserting correct setup)
-    cy.getTestElement(`page-a-link`).should(`not.exist`)
+    cy.findByTestId(`page-a-link`).should(`not.exist`)
     cy.go(`back`).waitForRouteChange()
   }
 

diff --git a/e2e-tests/development-runtime/cypress/integration/gatsby-preview/base-functionality.js b/e2e-tests/development-runtime/cypress/integration/gatsby-preview/base-functionality.js
@@ -9,6 +9,6 @@ describe(`Gatsby Preview, base functionality`, () => {
   })
 
   it(`displays correct data/id`, () => {
-    cy.get(`li`).invoke(`text`).should(`equal`, `Hello World (1)`)
+    cy.get(`li`).should(`have.text`, `Hello World (1)`)
   })
 })
diff --git a/e2e-tests/development-runtime/cypress/integration/hot-reloading/arrow-functions.js b/e2e-tests/development-runtime/cypress/integration/hot-reloading/arrow-functions.js
@@ -8,11 +8,9 @@ describe(`hot-reloading anonymous arrow functions`, () => {
     cy.visit(`/arrows`).waitForRouteChange()
   })
   it(`displays placeholders on launch`, () => {
-    cy.getTestElement(IDS.title).invoke(`text`).should(`contain`, `%TITLE%`)
+    cy.getTestElement(IDS.title).should(`have.text`, `%TITLE%`)
 
-    cy.getTestElement(IDS.subTitle)
-      .invoke(`text`)
-      .should(`contain`, `%SUB_TITLE%`)
+    cy.getTestElement(IDS.subTitle).should(`have.text`, `%SUB_TITLE%`)
   })
 
   it(`updates on change`, () => {
@@ -21,6 +19,6 @@ describe(`hot-reloading anonymous arrow functions`, () => {
       `npm run update -- --file src/components/title.tsx --replacements "TITLE:${text}"`
     )
 
-    cy.getTestElement(IDS.title).invoke(`text`).should(`eq`, text)
+    cy.getTestElement(IDS.title).should(`have.text`, text)
   })
 })
diff --git a/e2e-tests/development-runtime/cypress/integration/hot-reloading/class-component.js b/e2e-tests/development-runtime/cypress/integration/hot-reloading/class-component.js
@@ -5,9 +5,7 @@ describe(`reloading class component`, () => {
     cy.visit(`/`).waitForRouteChange()
   })
   it(`displays placeholder on launch`, () => {
-    cy.getTestElement(TEST_ID)
-      .invoke(`text`)
-      .should(`contain`, `%CLASS_COMPONENT%`)
+    cy.getTestElement(TEST_ID).should(`contain.text`, `%CLASS_COMPONENT%`)
   })
 
   it(`updates placeholder and hot reloads`, () => {
@@ -16,7 +14,7 @@ describe(`reloading class component`, () => {
       `npm run update -- --file src/components/class-component.js --replacements "CLASS_COMPONENT:${text}"`
     )
 
-    cy.getTestElement(TEST_ID).invoke(`text`).should(`contain`, text)
+    cy.getTestElement(TEST_ID).should(`contain.text`, text)
   })
 
   it(`updates state and hot reloads`, () => {
@@ -25,8 +23,9 @@ describe(`reloading class component`, () => {
       `npm run update -- --file src/components/class-component.js --replacements "CUSTOM_STATE:${value}"`
     )
 
-    cy.getTestElement(`stateful-${TEST_ID}`)
-      .invoke(`text`)
-      .should(`eq`, `Custom Message`)
+    cy.getTestElement(`stateful-${TEST_ID}`).should(
+      `have.text`,
+      `Custom Message`
+    )
   })
 })
diff --git a/e2e-tests/development-runtime/cypress/integration/hot-reloading/page-component.js b/e2e-tests/development-runtime/cypress/integration/hot-reloading/page-component.js
@@ -5,7 +5,7 @@ describe(`hot reloading page component`, () => {
     cy.visit(`/`).waitForRouteChange()
   })
   it(`displays placeholder content on launch`, () => {
-    cy.getTestElement(TEST_ID).invoke(`text`).should(`contain`, `%GATSBY_SITE%`)
+    cy.getTestElement(TEST_ID).should(`contain.text`, `%GATSBY_SITE%`)
   })
 
   it(`hot reloads with new content`, () => {
@@ -14,6 +14,6 @@ describe(`hot reloading page component`, () => {
       `npm run update -- --file src/pages/index.js --replacements "GATSBY_SITE:${text}"`
     )
 
-    cy.getTestElement(TEST_ID).invoke(`text`).should(`contain`, text)
+    cy.getTestElement(TEST_ID).should(`contain.text`, text)
   })
 })
diff --git a/e2e-tests/development-runtime/cypress/integration/hot-reloading/template-component.js b/e2e-tests/development-runtime/cypress/integration/hot-reloading/template-component.js
@@ -15,6 +15,6 @@ describe(`hot reloading template component`, () => {
       `npm run update -- --file src/templates/blog-post.js --replacements "${TEMPLATE}:${message}"`
     )
 
-    cy.getTestElement(TEST_ID).invoke(`text`).should(`eq`, `Hello ${message}`)
+    cy.getTestElement(TEST_ID).should(`have.text`, `Hello ${message}`)
   })
 })
diff --git a/e2e-tests/development-runtime/cypress/integration/navigation/linking.js b/e2e-tests/development-runtime/cypress/integration/navigation/linking.js
@@ -143,23 +143,46 @@ describe(`navigation`, () => {
     })
   })
 
-  describe(`All location changes should trigger an effect`, () => {
-    beforeEach(() => {
-      cy.visit(`/navigation-effects`).waitForRouteChange()
-    })
-
-    it(`should trigger an effect after a search param has changed`, () => {
-      cy.getTestElement(`effect-message`).invoke(`text`).should(`eq`, `Waiting for effect`)
-      cy.getTestElement(`send-search-message`).click().waitForRouteChange()
-      cy.getTestElement(`effect-message`).invoke(`text`).should(`eq`, `?message=searchParam`)
-    })
-
-    it(`should trigger an effect after the hash has changed`, () => {
-      cy.getTestElement(`effect-message`).invoke(`text`).should(`eq`, `Waiting for effect`)
-      cy.getTestElement(`send-hash-message`).click().waitForRouteChange()
-      cy.getTestElement(`effect-message`).invoke(`text`).should(`eq`, `#message-hash`)
-    })
-  })
+  if (Cypress.env("HOT_LOADER") !== `fast-refresh`) {
+    describe(`All location changes should trigger an effect (react-hot-loader)`, () => {
+      beforeEach(() => {
+        cy.visit(`/navigation-effects`).waitForRouteChange()
+      })
+
+      it(`should trigger an effect after a search param has changed`, () => {
+        cy.findByTestId(`effect-message`).should(`have.text`, `Waiting for effect`)
+        cy.findByTestId(`send-search-message`).click().waitForRouteChange()
+        cy.findByTestId(`effect-message`).should(`have.text`, `?message=searchParam`)
+      })
+
+      it(`should trigger an effect after the hash has changed`, () => {
+        cy.findByTestId(`effect-message`).should(`have.text`, `Waiting for effect`)
+        cy.findByTestId(`send-hash-message`).click().waitForRouteChange()
+        cy.findByTestId(`effect-message`).should(`have.text`, `#message-hash`)
+      })
+    })
+  }
+
+  // TODO: Check if this is the correct behavior
+  if (Cypress.env("HOT_LOADER") === `fast-refresh`) {
+    describe(`All location changes should trigger an effect (fast-refresh)`, () => {
+      beforeEach(() => {
+        cy.visit(`/navigation-effects`).waitForRouteChange()
+      })
+
+      it(`should trigger an effect after a search param has changed`, () => {
+        cy.findByTestId(`effect-message`).should(`have.text`, ``)
+        cy.findByTestId(`send-search-message`).click().waitForRouteChange()
+        cy.findByTestId(`effect-message`).should(`have.text`, `?message=searchParam`)
+      })
+
+      it(`should trigger an effect after the hash has changed`, () => {
+        cy.findByTestId(`effect-message`).should(`have.text`, ``)
+        cy.findByTestId(`send-hash-message`).click().waitForRouteChange()
+        cy.findByTestId(`effect-message`).should(`have.text`, `#message-hash`)
+      })
+    })
+  }
 
   describe(`Route lifecycle update order`, () => {
     it(`calls onPreRouteUpdate, render and onRouteUpdate the correct amount of times on route change`, () => {

diff --git a/e2e-tests/development-runtime/gatsby-config.js b/e2e-tests/development-runtime/gatsby-config.js
@@ -1,6 +1,11 @@
 // isomorphic-fetch sets global.fetch which seems to conflicts with source-map@<0.8.0 where it does a
 // simple browser check if (global.fetch) which is true when isomorphic-fetch is used. This creates an
 // exception in react-hot-loader. @see https://github.com/gatsbyjs/gatsby/pull/13713
+//
+// This is only necessary if we are testing react < 16.9 or if you are forcing GATSBY_HOT_LOADER=react-hot-loader.
+// When we are using fast-refresh we do not need this hack.
+// TODO: Remove once fast-refresh is the default
+//
 require(`isomorphic-fetch`)
 
 module.exports = {

diff --git a/e2e-tests/development-runtime/package.json b/e2e-tests/development-runtime/package.json
@@ -4,7 +4,7 @@
   "version": "1.0.0",
   "author": "Dustin Schau <[email protected]>",
   "dependencies": {
-    "gatsby": "^2.4.4",
+    "gatsby": "^2.27.1",
     "gatsby-image": "^2.0.41",
     "gatsby-plugin-image": "^0.0.2",
     "gatsby-plugin-manifest": "^2.0.17",
@@ -18,8 +18,8 @@
     "gatsby-transformer-sharp": "^2.1.19",
     "isomorphic-fetch": "^2.2.1",
     "prop-types": "^15.6.2",
-    "react": "16.8.6",
-    "react-dom": "16.8.6",
+    "react": "16.9.0",
+    "react-dom": "16.9.0",
     "react-helmet": "^5.2.1"
   },
   "keywords": [
@@ -30,6 +30,7 @@
   "scripts": {
     "build": "gatsby build",
     "develop": "cross-env CYPRESS_SUPPORT=y ENABLE_GATSBY_REFRESH_ENDPOINT=true gatsby develop",
+    "develop:fast-refresh": "cross-env CYPRESS_SUPPORT=y ENABLE_GATSBY_REFRESH_ENDPOINT=true GATSBY_HOT_LOADER=fast-refresh gatsby develop",
     "serve": "gatsby serve",
     "start": "npm run develop",
     "format": "prettier --write \"src/**/*.js\"",

diff --git a/e2e-tests/development-runtime/src/components/sub-title.js b/e2e-tests/development-runtime/src/components/sub-title.js
@@ -1,3 +1,7 @@
 import React from "react"
 
-export default () => <h2 data-testid="sub-title">{`%SUB_TITLE%`}</h2>
+export default function SubTitle() {
+  return (
+    <h2 data-testid="sub-title">{`%SUB_TITLE%`}</h2>
+  )
+}
diff --git a/e2e-tests/development-runtime/src/components/title.tsx b/e2e-tests/development-runtime/src/components/title.tsx
@@ -1,3 +1,7 @@
 import React from "react"
 
-export default () => <h1 data-testid="title">{`%TITLE%`}</h1>
+export default function Title() {
+  return (
+    <h1 data-testid="title">{`%TITLE%`}</h1>
+  )
+}
diff --git a/e2e-tests/development-runtime/src/pages/client-dynamic-route/[id].js b/e2e-tests/development-runtime/src/pages/client-dynamic-route/[id].js
@@ -1,9 +1,11 @@
 import React from "react"
 import Layout from "../../components/layout"
 
-export default props => (
+const ClientDynamicID = props => (
   <Layout>
     <h1 data-testid="title">Client Dynamic Route</h1>
     <h2 data-testid="params">{props.params.id}</h2>
   </Layout>
 )
+
+export default ClientDynamicID
diff --git a/e2e-tests/development-runtime/src/pages/client-dynamic-route/named-splat/[...name].js b/e2e-tests/development-runtime/src/pages/client-dynamic-route/named-splat/[...name].js
@@ -1,9 +1,11 @@
 import React from "react"
 import Layout from "../../../components/layout"
 
-export default props => (
+const NamedSplat = props => (
   <Layout>
     <h1 data-testid="title">Named SPLAT</h1>
     <h2 data-testid="splat">{props.params.name}</h2>
   </Layout>
 )
+
+export default NamedSplat
diff --git a/e2e-tests/development-runtime/src/pages/client-dynamic-route/products/[brand]/[product].js b/e2e-tests/development-runtime/src/pages/client-dynamic-route/products/[brand]/[product].js
@@ -1,10 +1,12 @@
 import React from "react"
 import Layout from "../../../../components/layout"
 
-export default props => (
+const NestedClientOnlyRoute = props => (
   <Layout>
     <h1 data-testid="title">Nested Dynamic Route</h1>
     <h2 data-testid="params-brand">{props.params.brand}</h2>
     <h2 data-testid="params-product">{props.params.product}</h2>
   </Layout>
 )
+
+export default NestedClientOnlyRoute
diff --git a/e2e-tests/development-runtime/src/pages/client-dynamic-route/splat/[...].js b/e2e-tests/development-runtime/src/pages/client-dynamic-route/splat/[...].js
@@ -1,9 +1,11 @@
 import React from "react"
 import Layout from "../../../components/layout"
 
-export default props => (
+const SplatRoute = props => (
   <Layout>
     <h1 data-testid="title">SPLAT!</h1>
     <h2 data-testid="splat">{props.params["*"]}</h2>
   </Layout>
 )
+
+export default SplatRoute
diff --git a/e2e-tests/development-runtime/src/pages/collection-routing/{FakeData.title}/[...name].js b/e2e-tests/development-runtime/src/pages/collection-routing/{FakeData.title}/[...name].js
@@ -1,9 +1,11 @@
 import React from "react"
 import Layout from "../../../components/layout"
 
-export default props => (
+const FakeDataTitleName = props => (
   <Layout>
     <h1 data-testid="title">Named SPLAT Nested with Collection Route!</h1>
     <h2 data-testid="splat">{props.params.name}</h2>
   </Layout>
 )
+
+export default FakeDataTitleName
diff --git a/e2e-tests/development-runtime/src/pages/queries-in-packages.js b/e2e-tests/development-runtime/src/pages/queries-in-packages.js
@@ -3,11 +3,13 @@ import SEO from "gatsby-seo"
 
 import Layout from "../components/layout"
 
-export default () => (
+const QueriesInPackages = () => (
   <Layout>
     <SEO
       title="Testing queries in packages"
       keywords={[`gatsby`, `application`, `react`, `queries in component`]}
     />
   </Layout>
 )
+
+export default QueriesInPackages
diff --git a/e2e-tests/development-runtime/src/utils/instrument-page.js b/e2e-tests/development-runtime/src/utils/instrument-page.js
@@ -1,6 +1,6 @@
 import React from "react"
 
-export default Page =>
+const InstrumentPage = Page =>
   class extends React.Component {
     addLogEntry(action) {
       if (typeof window !== `undefined`) {
@@ -31,3 +31,5 @@ export default Page =>
       return <Page {...this.props} />
     }
   }
+
+export default InstrumentPage