Migrating Vue components to React and stores refactoring #585

ixrock · 2020-07-15T14:14:27Z

Most noticeable changes:

updated electron to latest version
isomorphic stores (user-store, cluster-store, workspaces-store), sync via ipc
refactored vue parts to react (close Refactor Vue parts to React #396)
Fixes Get rid of ContextHandler.withTemporaryKubeconfig() #467
Fixes Open to last used workspace #348

In follow-up PRs

drag-n-drop for clusters in cluster-menu.tsx (fixed in add drag and drop capabilities for the order of cluster icons on the side bar #623)
add light-theme support for new pages (ex. vue)
allow user to upload kubeconfig (as done in Kubeconfigs as file references #466)
migration for cluster icons

Signed-off-by: Roman <ixrock@gmail.com>

# Conflicts: # src/common/cluster-store.ts # src/common/cluster-store_spec.ts

Signed-off-by: Roman <ixrock@gmail.com>

jim-docker · 2020-08-06T21:32:43Z

src/main/port.ts

+    });
+    server.on("error", error => {
+      logger.error(`Can't resolve new port: "${error}"`);
+      reject(error);


should there be a defensive server.close() here?

jim-docker · 2020-08-06T21:44:39Z

src/main/port_spec.ts

@@ -10,7 +12,10 @@ jest.mock("net", () => {
          return this
        })
        address = () => {
-          return { port: 12345 }
+          newPort = Math.round(Math.random() * 10000)


why use a random number in this test? (this could return 0, which although it would still pass the test, isn't a valid TCP port number)

* A bit of cleaning in Add Cluster page Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Adding head-col to WizardLayout Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Fixing Cluster Settings general layout bugs Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Cluster Status view refactoring Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Install Metrics component refactoring Using notifications for error, removed picking button icon method, simplified button generation. Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Remove icons / checks from RemoveClusterButton Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Fixing colorError in Input styles Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Preventing Input's spellchecking Signed-off-by: alexfront <alex.andreev.email@gmail.com> * ClusterNameSettings refactoring Signed-off-by: alexfront <alex.andreev.email@gmail.com> * ClusterWorkspaceSettings refactoring/fixing Signed-off-by: alexfront <alex.andreev.email@gmail.com> * ClusterProxySetting refactoring Signed-off-by: alexfront <alex.andreev.email@gmail.com> * ClusterPrometheusSetting refactoring Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Clean up Removal section Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Glued InstallMetrics & InstallUserMode into 1 component Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Removing unused styles in Cluster Settings Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Cluster Settings styling Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Adding close button to settings header Signed-off-by: alexfront <alex.andreev.email@gmail.com> * ClusterHomeDirSetting refactoring Signed-off-by: alexfront <alex.andreev.email@gmail.com> * FilePicker restyling Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Fixing Prometheus selector Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Fixing Hashicon Passing cluster name instead of cluster id to prevent icon changing while typing new cluster name Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Minor ClusterSettings fixes Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Increasing opacity for non-interactive icons Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Keep feature install loading state Waiting for props to change before disabling loading state (gray button width spinner) Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Remove arrays in disposeOnUnmount() Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Fix Cluster select behavior Now clicking cluster icon in sidebar always leads to / dashboard. And 'Settings' submenu switches active cluster at first and only the showing Cluster Settings Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Using structuralComparator in feature installer Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Saving input fields on blur Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Setting Select color same as Input color Signed-off-by: alexfront <alex.andreev.email@gmail.com>

Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>

Signed-off-by: alexfront <alex.andreev.email@gmail.com>

* Removing legacy browserCheck() utility Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Revert select box-shadow (outline) color Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Allowing to save prometheus service address Signed-off-by: alexfront <alex.andreev.email@gmail.com>

This prevents scrollbar flickering caused by Animation component with 'slide-left' appearing. Signed-off-by: alexfront <alex.andreev.email@gmail.com>

* Normalizing titles size and weight Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Replacing deployment scale icon Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Replacing download logs icon Signed-off-by: alexfront <alex.andreev.email@gmail.com> * Making <h4> tag bigger than <h5> Signed-off-by: alexfront <alex.andreev.email@gmail.com>

* add some simple user store tests Signed-off-by: Sebastian Malton <smalton@mirantis.com> Co-authored-by: Sebastian Malton <smalton@mirantis.com>

Nokel81 · 2020-08-12T17:35:04Z

src/common/base-store.ts

+      syncEnabled: true,
+      ...params,
+    }
+    this.init();


I think this call is fundamentally broken. It is not awaited (so when the getInstance call finishes the store isn't even up to date) and the default params to init block from enabling sync.

Especially since the load is just called later anyway.

Wat? :) can you elablorate/provide more examples?
When you call getInstance() you always have an instance of the store.
Yes, it might be not loaded with a data yet because of async load() and it doesn't mean it is ready-to-use immediately. Every store has isLoaded field which takes in account before app ready to start.

I mean that it seems like a massive foot gun to get one of these and not be able to get the correct data immediately afterwards.

What I mean is that even if you pass into getInstance the argument { parms: { autoLoad: true } } it won't be correct.

Further more, in the default case it seems weird for the unbounded promise (which in future versions of node will abort the process if it throws) halts on await this.whenLoaded. I understand that the point of this is so that enableSync can be started only once the store has loaded but the current method seems clunky.

not be able to get the correct data immediately afterwards

how you can get it immediately for async store loading? (yes, currently it can be loaded sync from file-system, but it was developed to support async load)

didn't get what you mean about await this.whenLoaded and what's wrong with it?
electron has similar stuff, like app.whenReady

jakolehm · 2020-08-13T05:35:42Z

src/common/base-store.ts

+  * [Symbol.iterator]() {
+    yield* Object.entries(this.toJSON());
+  }


If it's not used anywhere why we added it? Sounds like a dead code to me 🤔

jakolehm · 2020-08-13T05:37:23Z

src/common/kube-helpers.ts

+}
+
+// Logic adapted from dashboard
+// see: https://github.com/kontena/kontena-k8s-dashboard/blob/7d8f9cb678cc817a22dd1886c5e79415b212b9bf/client/api/endpoints/nodes.api.ts#L147


This link is not publicly accessible, we should remove it.

jakolehm · 2020-08-13T05:38:35Z

src/common/tracker.ts

-    const userPrefs = userStore.getPreferences()
-    return !!userPrefs.allowTelemetry
+    } catch (err) {
+      console.error(`Failed to track "${eventCategory}:${eventAction}"`, err)


Should this use logger?

jakolehm · 2020-08-13T05:41:43Z

src/common/workspace-store.ts

+  @action
+  protected fromStore({ currentWorkspace, workspaces = [] }: WorkspaceStoreModel) {
+    if (currentWorkspace) {
+      this.currentWorkspaceId = currentWorkspace


I agree with @jim-docker here, this code is a bit confusing if it first expects that there are always workspaces and then it still checks if there are none. Maybe stop hard if we detect invalid state (== no workspaces)?

jakolehm · 2020-08-13T05:45:31Z

src/main/context-handler.ts

-    this.contextName = cluster.contextName;
-    this.defaultNamespace = kc.getContextObject(cluster.contextName).namespace
-    this.url = `http://${this.id}.localhost:${cluster.port}/`
-    this.kubernetesApi = `http://127.0.0.1:${cluster.port}/${this.id}`


How these changes are related to Vue->React refactoring (I mean most changes in this file)? I'm a bit afraid that these will cause troubles without more testing.

Vue->React + stores + overall design requires this changes..

jakolehm · 2020-08-13T05:52:34Z

src/main/port.ts

-    throw("getNextAvailablePort() threw: '" + error + "'")
-  }
-  return freePort
+export async function getFreePort(): Promise<number> {


Same here, how this is related to Vue->React refactoring?

It's not like normal converting single vue->react component :)
so please percept this like almost kick-starting new app with extra refactoring (only where it's simple or neccesary)

jakolehm · 2020-08-13T06:01:33Z

package.json

    "electron-builder": "^22.7.0",
    "electron-notarize": "^0.3.0",
+    "electron-rebuild": "^1.11.0",


electron-rebuild not required if you use electron-builder, please consider to remove excess dependency from devDependencies

I think we should remove this.

this is used in yarn rebuild-pty (this commands helps when switching electron versions)

i'm wondering why it's caring about devDependencies 🤔

Signed-off-by: Roman <ixrock@gmail.com>

* add some basic workspace store tests Signed-off-by: Sebastian Malton <smalton@mirantis.com> Co-authored-by: Sebastian Malton <smalton@mirantis.com>

* Views management refactoring Signed-off-by: Roman <ixrock@gmail.com> Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>

Signed-off-by: Roman <ixrock@gmail.com>

ixrock added 30 commits July 3, 2020 14:15

store migrations refactoring

d7cffeb

Signed-off-by: Roman <ixrock@gmail.com>

use common/utils/singleton class for the stores

776fc44

Signed-off-by: Roman <ixrock@gmail.com>

clean up

e7975ec

Signed-off-by: Roman <ixrock@gmail.com>

Merge remote-tracking branch 'origin/master' into vue_react_migration

f9c0c5d

# Conflicts: # src/common/cluster-store.ts # src/common/cluster-store_spec.ts

master-merge fixes

c9fb015

Signed-off-by: Roman <ixrock@gmail.com>

tracker / user-store refactoring, part 1

4270ec4

Signed-off-by: Roman <ixrock@gmail.com>

added "logger" to user-store

deea97e

Signed-off-by: Roman <ixrock@gmail.com>

fix errors

387bc07

Signed-off-by: Roman <ixrock@gmail.com>

user-store: more debug logging

9d42b2c

Signed-off-by: Roman <ixrock@gmail.com>

user-store: more fixes

397d7c5

Signed-off-by: Roman <ixrock@gmail.com>

user-store: more fixes 2

9244e8f

Signed-off-by: Roman <ixrock@gmail.com>

incorrect seen context update -- fixed vue-store mutation

5c8dd89

Signed-off-by: Roman <ixrock@gmail.com>

use common/base-store for config-file based stores (e.g. user-store)

a27ce9c

Signed-off-by: Roman <ixrock@gmail.com>

workspace-store refactoring

d4bbaf4

Signed-off-by: Roman <ixrock@gmail.com>

cluster-store refactoring -- part 1

afb5469

Signed-off-by: Roman <ixrock@gmail.com>

fix type imports

9f97015

Signed-off-by: Roman <ixrock@gmail.com>

cluster-store refactoring -- part 2

17cd886

Signed-off-by: Roman <ixrock@gmail.com>

added common/ipc-helpers

bde60b1

Signed-off-by: Roman <ixrock@gmail.com>

cluster-store refactoring -- part 3

f474a20

Signed-off-by: Roman <ixrock@gmail.com>

added tracking cluster-manager's ipc events

cf00deb

Signed-off-by: Roman <ixrock@gmail.com>

epic: getting rid of vue -- part 1

63de5fc

Signed-off-by: Roman <ixrock@gmail.com>

epic: getting rid of vue -- part 2

1f26f0b

Signed-off-by: Roman <ixrock@gmail.com>

added dummy react files for replacing vue-components

434f26f

Signed-off-by: Roman <ixrock@gmail.com>

more react dummy files

b9089a7

Signed-off-by: Roman <ixrock@gmail.com>

added workspaces layout, more refactoring

4a06f65

Signed-off-by: Roman <ixrock@gmail.com>

refactoring

507e2dc

Signed-off-by: Roman <ixrock@gmail.com>

workspaces: extract bottom-bar to separated component

ba4f5cb

Signed-off-by: Roman <ixrock@gmail.com>

clusters-menu

a440f60

Signed-off-by: Roman <ixrock@gmail.com>

upgrade electron to latest version

ce1cccc

Signed-off-by: Roman <ixrock@gmail.com>

window-manager refactoring

cac4896

Signed-off-by: Roman <ixrock@gmail.com>

Nokel81 mentioned this pull request Aug 6, 2020

Master donut graph for memory usage only appears to show one master #594

Closed

jim-docker reviewed Aug 7, 2020

View reviewed changes

Nokel81 linked an issue Aug 7, 2020 that may be closed by this pull request

Master donut graph for memory usage only appears to show one master #594

Closed

nevalla and others added 8 commits August 10, 2020 09:03

Merge branch 'master' into vue_react_migration

efcf02e

Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>

Removing legacy browserCheck() utility

5c1ded6

Signed-off-by: alexfront <alex.andreev.email@gmail.com>

Revert select box-shadow (outline) color

0393d1f

Signed-off-by: alexfront <alex.andreev.email@gmail.com>

Allowing to save prometheus service address

de46907

Signed-off-by: alexfront <alex.andreev.email@gmail.com>

Improving code readabilty

ec1ed63

Signed-off-by: alexfront <alex.andreev.email@gmail.com>

Setting overflow:hidden on lens-view elem (#664)

f7e823d

This prevents scrollbar flickering caused by Animation component with 'slide-left' appearing. Signed-off-by: alexfront <alex.andreev.email@gmail.com>

ixrock changed the title ~~[WIP]: Migrating Vue components to React and stores refactoring~~ Migrating Vue components to React and stores refactoring Aug 11, 2020

add some simple user store tests (#674)

5fc56da

* add some simple user store tests Signed-off-by: Sebastian Malton <smalton@mirantis.com> Co-authored-by: Sebastian Malton <smalton@mirantis.com>

Nokel81 reviewed Aug 12, 2020

View reviewed changes

jakolehm reviewed Aug 13, 2020

View reviewed changes

Nokel81 linked an issue Aug 13, 2020 that may be closed by this pull request

node-shell pods are pending #192

Closed

ixrock and others added 6 commits August 14, 2020 15:21

PR comment fixes for @Jari

6725a6f

Signed-off-by: Roman <ixrock@gmail.com>

add some basic workspace store tests (#680)

592c892

* add some basic workspace store tests Signed-off-by: Sebastian Malton <smalton@mirantis.com> Co-authored-by: Sebastian Malton <smalton@mirantis.com>

Views management refactoring (#669)

47bcb63

* Views management refactoring Signed-off-by: Roman <ixrock@gmail.com> Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>

Merge branch 'master' into vue_react_migration

fa4407b

fix: #draggable-top (devtools must be closed on drag)

1f890f1

Signed-off-by: Roman <ixrock@gmail.com>

fix: doubled "copyright" in package.json after master-merge

06b7322

Signed-off-by: Roman <ixrock@gmail.com>

nevalla approved these changes Aug 19, 2020

View reviewed changes

aleksfront mentioned this pull request Aug 19, 2020

Handle status values that contains an object #693

Merged

nevalla merged commit 5670312 into master Aug 20, 2020

nevalla deleted the vue_react_migration branch August 20, 2020 06:02

nevalla restored the vue_react_migration branch August 20, 2020 06:04

nevalla deleted the vue_react_migration branch August 27, 2020 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrating Vue components to React and stores refactoring #585

Migrating Vue components to React and stores refactoring #585

ixrock commented Jul 15, 2020 •

edited

Loading

jim-docker Aug 6, 2020

jim-docker Aug 6, 2020

Nokel81 Aug 12, 2020 •

edited

Loading

ixrock Aug 12, 2020

Nokel81 Aug 12, 2020

ixrock Aug 14, 2020

jakolehm Aug 13, 2020

jakolehm Aug 13, 2020

ixrock Aug 14, 2020

jakolehm Aug 13, 2020

ixrock Aug 14, 2020

jakolehm Aug 13, 2020

jakolehm Aug 13, 2020

ixrock Aug 14, 2020 •

edited

Loading

jakolehm Aug 13, 2020

ixrock Aug 14, 2020 •

edited

Loading

jakolehm Aug 13, 2020

ixrock Aug 14, 2020

ixrock Aug 14, 2020

Migrating Vue components to React and stores refactoring #585

Migrating Vue components to React and stores refactoring #585

Conversation

ixrock commented Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nokel81 Aug 12, 2020 • 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixrock Aug 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixrock Aug 14, 2020 • 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

ixrock commented Jul 15, 2020 •

edited

Loading

Nokel81 Aug 12, 2020 •

edited

Loading

ixrock Aug 14, 2020 •

edited

Loading

ixrock Aug 14, 2020 •

edited

Loading