log-viewer-webui: Add boilerplate React client. #468

kirkrodrigues · 2024-07-03T10:48:31Z

Description

Following #442, this PR adds a boilerplate client written using React.

For consistency, the server's package.json was also changed to:

Rename the package to log-viewer-webui-server
Change the dev-server target to npm run watch

Validation performed

Validated npm lint:check passes.
Validated npm run watch starts a local HTTP server that keeps track of source changes.
Validated npm run build builds a bundle in dist that can be served with npx http-server dist

junhaoliao

In general it looks good to me. There are some devDependencies that are specified but I do not see the usages of those in the webpack config file. If those are to be configured later, can we submit those another PR?

junhaoliao · 2024-07-04T20:18:59Z

components/log-viewer-webui/client/package.json

+    "lint:check": "npx eslint --no-eslintrc --config package.json src webpack.config.js",
+    "lint:fix": "npx eslint --fix --no-eslintrc --config package.json src webpack.config.js",
+    "build": "webpack --define-process-env-node-env production",
+    "watch": "webpack serve"


For consistency, can we rename this config as start? Let's continue the discussion in server/package.json.

junhaoliao · 2024-07-04T20:20:07Z

components/log-viewer-webui/server/package.json

  "version": "0.1.0",
  "description": "",
  "main": "src/main.js",
  "scripts": {
-    "dev": "NODE_ENV=development nodemon src/main.js",
-    "lint:check": "npx eslint --no-eslintrc --config package.json src",
-    "lint:fix": "npx eslint --fix --no-eslintrc --config package.json src",
    "start": "NODE_ENV=production node src/main.js",


Would it be better to rename this script as prod and the one below for development as start? That way we will be consistently using start as the name for all development scripts across all repos.

junhaoliao · 2024-07-04T20:27:11Z

components/log-viewer-webui/client/package.json

+    "@babel/preset-env": "^7.24.7",
+    "@babel/preset-react": "^7.24.7",
+    "@webpack-cli/generators": "^3.0.7",
+    "autoprefixer": "^10.4.19",


This seems handy, but it seems not only we need to configure "browserslist" in this package.json file but also we need to install postcss and add some related config in webpack.config.js according to their docs: https://github.com/postcss/autoprefixer#webpackhttps://github.com/postcss/autoprefixer#webpack
(Did I miss anything? )

Sorry, I did have postcss initially, but had some issues so I removed it but forgot to remove this.

junhaoliao · 2024-07-04T20:32:24Z

components/log-viewer-webui/client/package.json

+    "@babel/plugin-proposal-class-properties": "^7.18.6",
+    "@babel/plugin-transform-runtime": "^7.24.7",


I don't see those getting configured in webpack.config.js. Mind sharing what the usages are of those?

Sorry, didn't clean up package.json properly.

junhaoliao · 2024-07-04T20:32:37Z

components/log-viewer-webui/client/package.json

+    "@babel/plugin-transform-runtime": "^7.24.7",
+    "@babel/preset-env": "^7.24.7",
+    "@babel/preset-react": "^7.24.7",
+    "@webpack-cli/generators": "^3.0.7",


Ditto - why do we need this?

Maybe we just need "webpack-cli": "^5.1.4",?

junhaoliao · 2024-07-04T20:33:52Z

components/log-viewer-webui/client/package.json

+    "eslint-config-yscope": "latest",
+    "html-webpack-plugin": "^5.6.0",
+    "mini-css-extract-plugin": "^2.9.0",
+    "prettier": "^3.3.2",


Do we need to add a config file for this?

junhaoliao · 2024-07-04T20:35:52Z

components/log-viewer-webui/client/webpack.config.js

+];
+
+const config = {
+    devtool: isProduction ?


Is the devServer config to be added in a future PR?

Maybe I read the docs incorrectly, but it seemed like the defaults worked?

Right, the defaults do work, but do we want to be explicit about which port to use? Also, similar to HMR, do we want to enable "Fast Refresh" with https://github.com/pmmmwh/react-refresh-webpack-plugin ?

I think the port defaults to 'auto' which seems like it would be more flexible than specifying it explicitly (and the CLI prints out what it chose).

We could add open if you like (I prefer not automatically opening the browser, but that's just me).

What does that plugin do compared to the default dev server?

I think the port defaults to 'auto' which seems like it would be more flexible than specifying it explicitly (and the CLI prints out what it chose).

Gotcha. Let's leave it as is then.

We could add open if you like (I prefer not automatically opening the browser, but that's just me).

I don't have a preference so I'm fine leaving it as false.

What does that plugin do compared to the default dev server?

It's for preserving states of any modified React components. e.g. Say we are modifying a button in a modal window, with conventional HMR the modal window will be closed when the page auto reloads. With Fast Refresh, the modal window will remain open without us clicking again in the browser. Another example is with Fast Refresh, any existing text input is preserved in a text field when the page auto reloads.

Gotcha, thanks.

junhaoliao · 2024-07-04T20:37:31Z

components/log-viewer-webui/client/webpack.config.js

+    },
+    output: {
+        path: path.resolve(dirname, "dist"),
+        filename: "[name].[contenthash].bundle.js",


The contenthash has been causing issues with HMR / Fast Refresh in my experience and I recommend removing such in a dev environment setting.

Suggested change

filename: "[name].[contenthash].bundle.js",

filename: isProduction?

"[name].[contenthash].bundle.js":

"[name].bundle.js",

components/log-viewer-webui/client/package.json

junhaoliao · 2024-07-05T00:35:20Z

components/log-viewer-webui/server/package.json

  "version": "0.1.0",
  "description": "",
  "main": "src/main.js",
  "scripts": {
-    "dev": "NODE_ENV=development nodemon src/main.js",
+    "prod": "NODE_ENV=production node src/main.js",


Let's alphabetize these.

docs/src/dev-guide/components-log-viewer-webui.md

Co-authored-by: Junhao Liao <[email protected]>

components/log-viewer-webui/client/package.json

Co-authored-by: Junhao Liao <[email protected]>

junhaoliao

Looks good to me. The PR title is fine for the squashed commit message.

Co-authored-by: Junhao Liao <[email protected]>

log-viewer-webui: Add boilerplate React client.

aafaa1b

kirkrodrigues requested a review from junhaoliao July 3, 2024 10:48

junhaoliao reviewed Jul 4, 2024

View reviewed changes

kirkrodrigues added 3 commits July 4, 2024 17:00

Don't use contenthash in dev mode.

b81dac3

Make npm scripts consistent.

2b30a1a

Remove unnecessary deps.

102553a

kirkrodrigues requested a review from junhaoliao July 4, 2024 21:05

junhaoliao requested changes Jul 5, 2024

View reviewed changes

kirkrodrigues and others added 3 commits July 4, 2024 20:41

Update components/log-viewer-webui/client/package.json

0b12320

Co-authored-by: Junhao Liao <[email protected]>

Update docs/src/dev-guide/components-log-viewer-webui.md

4acb732

Co-authored-by: Junhao Liao <[email protected]>

Update docs/src/dev-guide/components-log-viewer-webui.md

12d2f24

Co-authored-by: Junhao Liao <[email protected]>

junhaoliao reviewed Jul 5, 2024

View reviewed changes

components/log-viewer-webui/client/package.json Outdated Show resolved Hide resolved

kirkrodrigues and others added 5 commits July 4, 2024 20:44

Update package.json

7ab254d

Update components/log-viewer-webui/client/package.json

1f07d3a

Co-authored-by: Junhao Liao <[email protected]>

docs: Update server's running in prod command.

65b12d9

Remove dangling comma.

fa98963

Use ternary operator.

fe01d12

kirkrodrigues requested a review from junhaoliao July 5, 2024 02:29

Use react-refresh to enable React-aware hot module reloading.

a756719

kirkrodrigues requested review from junhaoliao and removed request for junhaoliao July 5, 2024 05:00

junhaoliao approved these changes Jul 5, 2024

View reviewed changes

kirkrodrigues merged commit 4bb223e into y-scope:main Jul 5, 2024
4 checks passed

kirkrodrigues deleted the log-viewer-webui-client branch July 5, 2024 05:02

kirkrodrigues mentioned this pull request Jul 6, 2024

log-viewer-webui: Serve client using server in production; Add npm scripts to setup, lint, and start client and server together during development. #474

Merged

jackluo923 pushed a commit to jackluo923/clp that referenced this pull request Dec 4, 2024

log-viewer-webui: Add boilerplate React client. (y-scope#468)

1f0b637

Co-authored-by: Junhao Liao <[email protected]>

junhaoliao mentioned this pull request Dec 27, 2024

chore(log-viewer-webui-client): Convert the codebase to TypeScript; Update dependencies. #645

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

log-viewer-webui: Add boilerplate React client. #468

log-viewer-webui: Add boilerplate React client. #468

kirkrodrigues commented Jul 3, 2024

junhaoliao left a comment

junhaoliao Jul 4, 2024

junhaoliao Jul 4, 2024

junhaoliao Jul 4, 2024

kirkrodrigues Jul 4, 2024

junhaoliao Jul 4, 2024 •

edited

Loading

kirkrodrigues Jul 4, 2024

junhaoliao Jul 4, 2024

junhaoliao Jul 4, 2024

junhaoliao Jul 4, 2024

kirkrodrigues Jul 4, 2024

junhaoliao Jul 4, 2024

kirkrodrigues Jul 4, 2024

junhaoliao Jul 5, 2024

kirkrodrigues Jul 5, 2024

junhaoliao Jul 5, 2024 •

edited

Loading

kirkrodrigues Jul 5, 2024

junhaoliao Jul 4, 2024

junhaoliao Jul 5, 2024

junhaoliao left a comment

		"@babel/plugin-proposal-class-properties": "^7.18.6",
		"@babel/plugin-transform-runtime": "^7.24.7",

-        filename: "[name].[contenthash].bundle.js",
+        filename: isProduction?
+            "[name].[contenthash].bundle.js":
+            "[name].bundle.js",

log-viewer-webui: Add boilerplate React client. #468

log-viewer-webui: Add boilerplate React client. #468

Conversation

kirkrodrigues commented Jul 3, 2024

Description

Validation performed

junhaoliao left a 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

junhaoliao Jul 4, 2024 • 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

junhaoliao Jul 5, 2024 • 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

junhaoliao left a comment

Choose a reason for hiding this comment

junhaoliao Jul 4, 2024 •

edited

Loading

junhaoliao Jul 5, 2024 •

edited

Loading