add react-refresh hot reloading #5246

glekner · 2020-04-30T14:36:28Z

This adds the react-refresh webpack plugin, which makes hot component reloading really work.
My webpack PR from 8 months ago enabled an auto reload, with hot reload we won't even need to reload.
This is very beneficial for us because of websockets, they remain in tact so we don't have to wait for a cluster far away, therefore we can develop and debug a LOT faster.

This won't work for all file changes, and once they don't - a full reload is triggered ( just like the current behaviour)
files that won't hot reload:

components/selectors/constants that are exported like this export * from
combining a component and render logic in the same file (just like our very own app.jsx)

@spadgett @alecmerdler thoughts?

spadgett · 2020-04-30T14:38:49Z

/cc @christianvogt @vojtechszocs

christianvogt · 2020-04-30T15:31:37Z

Sounds like a win to me.
I miss the days of working on projects with hot reloading that doesn't refresh the page.

alecmerdler · 2020-04-30T16:30:39Z

frontend/package.json

@@ -162,6 +167,7 @@
    "@types/react-transition-group": "2.x",
    "@types/react-virtualized": "9.x",
    "@types/webpack": "4.x",
+    "babel-loader": "^8.1.0",


We need Babel now on top of TypeScript?

christianvogt · 2020-04-30T16:37:29Z

frontend/webpack.config.ts

+          {
+            loader: 'babel-loader',
+            options: {
+              plugins: [isDevelopment && 'react-refresh/babel'],
+            },
+          },


babel-loader should only be added if isDevelopment === true

christianvogt · 2020-04-30T16:38:09Z

Are there any performance changes with using babel now in dev startup and recompiling?

glekner · 2020-05-03T08:30:27Z

@christianvogt @alecmerdler
fixed suggestions.
We do need babel now with this plugin but it doesn't hurt performance at all.
Compile and recompile time is the same for me.
from @pmmmwh plugin:

If you are using TypeScript (instead of Babel) as a transpiler, you will still need to use babel-loader to process your source code.

Without PR changes:

With PR changes:

glekner · 2020-05-11T09:52:48Z

updated packages

spadgett · 2020-05-11T13:26:03Z

@glekner Thanks for the PR. I'm on board.

You have a Webpack error, though.

Invalid configuration object. Webpack has been initialised using a configuration object that does not match the API schema.
 - configuration.module.rules[1].use should be one of these:
   non-empty string | function | object { ident?, loader?, options?, query? } | function | [non-empty string | function | object { ident?, loader?, options?, query? }]
   -> Modifiers applied to the module when rule is matched
   Details:
    * configuration.module.rules[1].use[2] should be a string.
    * configuration.module.rules[1].use[2] should be an instance of function
    * configuration.module.rules[1].use[2] should be an object.
    * configuration.module.rules[1].use[2] should be one of these:
      non-empty string | function | object { ident?, loader?, options?, query? }
    * configuration.module.rules[1].use[2] should be one of these:
      non-empty string | function | object { ident?, loader?, options?, query? }
      -> An use item
 - configuration.plugins[7] should be one of these:
   object { apply, … } | function
   -> Plugin of type object or instanceof Function
   Details:
    * configuration.plugins[7] should be an object.
      -> Plugin instance
    * configuration.plugins[7] should be an instance of function
      -> Function acting as plugin
error Command failed with exit code 1.

glekner · 2020-05-11T22:01:46Z

/retest

glekner · 2020-05-11T22:04:13Z

@spadgett this is weird, my env works well.
can you try

removing node modules and reinstalling
installing webpack-dev-server globally
and try again?

spadgett · 2020-05-12T13:19:23Z

@glekner The error is not in my environment. It's in CI (which uses a fresh workspace).

Did you try ./build-frontend.sh as opposed to yarn run dev?

spadgett · 2020-05-12T13:22:37Z

frontend/webpack.config.ts

@@ -68,6 +70,12 @@ const config: Configuration = {
              workers: require('os').cpus().length - 1,
            },
          },
+          isDevelopment && {


This looks like the cause of the error. Won't this add false to the array when not in development mode?

glekner · 2020-05-13T10:25:48Z

@spadgett

Added filter(Boolean) to rules, plugins arrays to fix production build

frontend/webpack.config.ts

glekner · 2020-05-17T12:29:52Z

@spadgett removed .filter(Boolean)

Also created #5467
As a first step towards 100% hot module replacement.

spadgett

/approve
/lgtm

glekner · 2020-05-25T11:11:03Z

@spadgett upgraded plugin version

frontend/webpack.config.ts

spadgett

/lgtm

openshift-ci-robot · 2020-05-26T14:58:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: glekner, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-05-26T16:23:09Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-05-26T17:41:18Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot requested review from jhadvig and rhamilto April 30, 2020 14:36

glekner changed the title ~~Experimental - add react-refresh hot reloading~~ add react-refresh hot reloading Apr 30, 2020

openshift-ci-robot requested review from christianvogt and vojtechszocs April 30, 2020 14:38

alecmerdler reviewed Apr 30, 2020

View reviewed changes

christianvogt reviewed Apr 30, 2020

View reviewed changes

glekner force-pushed the react-refresh-lite branch from c73125f to 6c2e375 Compare May 3, 2020 08:31

glekner force-pushed the react-refresh-lite branch from 6c2e375 to bcfe0d9 Compare May 11, 2020 09:52

openshift-ci-robot added component/core Related to console core functionality component/dev-console Related to dev-console component/knative Related to knative-plugin component/kubevirt Related to kubevirt-plugin component/olm Related to OLM labels May 11, 2020

glekner force-pushed the react-refresh-lite branch from b821cb8 to eb81783 Compare May 11, 2020 22:25

spadgett reviewed May 12, 2020

View reviewed changes

glekner force-pushed the react-refresh-lite branch 4 times, most recently from 8dde73a to e694fc3 Compare May 13, 2020 10:23

spadgett reviewed May 13, 2020

View reviewed changes

frontend/webpack.config.ts Outdated Show resolved Hide resolved

spadgett reviewed May 13, 2020

View reviewed changes

frontend/webpack.config.ts Outdated Show resolved Hide resolved

glekner force-pushed the react-refresh-lite branch from e694fc3 to a38ee43 Compare May 17, 2020 12:17

glekner mentioned this pull request May 17, 2020

Decouple Render and App component #5467

Closed

4 tasks

spadgett approved these changes May 20, 2020

View reviewed changes

openshift-ci-robot assigned spadgett May 20, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 20, 2020

glekner force-pushed the react-refresh-lite branch from a38ee43 to faa7ff4 Compare May 25, 2020 11:10

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 25, 2020

glekner force-pushed the react-refresh-lite branch from faa7ff4 to 190a6f1 Compare May 25, 2020 11:12

spadgett reviewed May 26, 2020

View reviewed changes

frontend/webpack.config.ts Outdated Show resolved Hide resolved

add react refresh

a4b1561

glekner force-pushed the react-refresh-lite branch from 190a6f1 to a4b1561 Compare May 26, 2020 14:39

spadgett approved these changes May 26, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2020

openshift-merge-robot merged commit 0b55e38 into openshift:master Jun 1, 2020

spadgett added this to the v4.6 milestone Jun 1, 2020

spadgett mentioned this pull request Jun 1, 2020

Revert "add react-refresh hot reloading" #5640

Merged

spadgett added the kind/dependency-change Categorizes issue or PR as related to changing dependencies label Jul 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add react-refresh hot reloading #5246

add react-refresh hot reloading #5246

glekner commented Apr 30, 2020

spadgett commented Apr 30, 2020

christianvogt commented Apr 30, 2020

alecmerdler Apr 30, 2020

christianvogt Apr 30, 2020

christianvogt commented Apr 30, 2020

glekner commented May 3, 2020

glekner commented May 11, 2020

spadgett commented May 11, 2020

glekner commented May 11, 2020

glekner commented May 11, 2020

spadgett commented May 12, 2020

spadgett May 12, 2020

glekner commented May 13, 2020

glekner commented May 17, 2020

spadgett left a comment

glekner commented May 25, 2020

spadgett left a comment

openshift-ci-robot commented May 26, 2020

openshift-bot commented May 26, 2020

openshift-bot commented May 26, 2020

add react-refresh hot reloading #5246

add react-refresh hot reloading #5246

Conversation

glekner commented Apr 30, 2020

spadgett commented Apr 30, 2020

christianvogt commented Apr 30, 2020

alecmerdler Apr 30, 2020

Choose a reason for hiding this comment

christianvogt Apr 30, 2020

Choose a reason for hiding this comment

christianvogt commented Apr 30, 2020

glekner commented May 3, 2020

glekner commented May 11, 2020

spadgett commented May 11, 2020

glekner commented May 11, 2020

glekner commented May 11, 2020

spadgett commented May 12, 2020

spadgett May 12, 2020

Choose a reason for hiding this comment

glekner commented May 13, 2020

glekner commented May 17, 2020

spadgett left a comment

Choose a reason for hiding this comment

glekner commented May 25, 2020

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented May 26, 2020

openshift-bot commented May 26, 2020

openshift-bot commented May 26, 2020