log-viewer-webui: Add boilerplate React client. by kirkrodrigues · Pull Request #468 · y-scope/clp

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

+    "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

-    "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

+    "@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

+    "@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

+    "@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

+    "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

+];
+
+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

+    },
+    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",

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

  "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.

Co-authored-by: Junhao Liao <junhao@junhao.ca>

junhaoliao

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

Co-authored-by: Junhao Liao <junhao@junhao.ca>

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 <junhao@junhao.ca>

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

4acb732

Co-authored-by: Junhao Liao <junhao@junhao.ca>

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

12d2f24

Co-authored-by: Junhao Liao <junhao@junhao.ca>

junhaoliao reviewed Jul 5, 2024

View reviewed changes

Comment thread components/log-viewer-webui/client/package.json Outdated

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 <junhao@junhao.ca>

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

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 <junhao@junhao.ca>

junhaoliao mentioned this pull request Dec 27, 2024

build(log-viewer-webui-client): Switch from Webpack to Vite for bundling; Convert the codebase to TypeScript. #645

Merged

junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026

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

588dffd

Co-authored-by: Junhao Liao <junhao@junhao.ca>

		"@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",

Conversation

kirkrodrigues commented Jul 3, 2024

Description

Validation performed

Uh oh!

junhaoliao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junhaoliao Jul 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junhaoliao Jul 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

junhaoliao left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

junhaoliao Jul 4, 2024 •

edited

Loading

junhaoliao Jul 5, 2024 •

edited

Loading