-
Notifications
You must be signed in to change notification settings - Fork 111
refresh worker-react example
#127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refresh worker-react example
#127
Conversation
ExecutionContext was missing in worker.ts. it's actually unused, but I was thinking that it was probably here (since this is an example) to show it, so instead of deleting it to remove the type error I fixed the problem by adding a tsconfig referencing the types which are present in the root. if it'd be better to give this example its own package.json to install the cloudflare worker types, I'm happy to update! lemme know
This prevents unnecessary function recreations on every render, which is especially important since runDemo is passed to the button's onClick handler.
conditional rendering using && can cause issues in React because when left side is falsy, it renders that falsy value (like false, null, 0). although code like this used to be common, people use ternaries with explicit null these days, considering it safer.
I tried to keep it as non-invasive as possible, but in order to make the chart responsive I switched from SVG to plain divs (that will become important in the next commit where I will scale both)
in the summary, it would be nice to have a side-by-side horizontal histogram
|
|
I have read the CLA Document and I hereby sign the CLA dimitropoulos seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refreshes the worker-react example with modern styling, dark mode support, and improved user experience. The changes include upgrading dependencies, refactoring the UI from SVG-based charts to responsive HTML/CSS flex-based charts, and improving the visual design with a cohesive theme.
Changes:
- Upgraded React from 18.3.1 to 19.2.3, along with associated dependencies (Vite, TypeScript, esbuild)
- Refactored TraceView component from SVG to responsive HTML/CSS with flexbox layout
- Added dark mode support with CSS custom properties and prefers-color-scheme media queries
- Improved conditional rendering to wait for both test results before displaying
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/worker-react/web/src/main/App.tsx | Refactored to use useCallback hooks, improved conditional rendering, and updated TraceView to use maxTime for consistent chart scaling |
| examples/worker-react/web/src/main/App.css | New file with comprehensive styling including theme variables, dark mode support, and responsive chart layout |
| examples/worker-react/web/package.json | Updated dependencies: React 19.2.3, Vite 7.3.1, TypeScript 5.9.3 |
| examples/worker-react/web/package-lock.json | Updated lockfile with all dependency version changes and their transitive dependencies |
| examples/worker-react/tsconfig.json | New root-level TypeScript configuration for the worker code |
Files not reviewed (1)
- examples/worker-react/web/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
examples/worker-react/web/package-lock.json:1069
- Vite version 7.3.1 appears to be a non-existent version. As of January 2025, Vite's latest stable version should be in the 5.x or 6.x range. This version may not exist on npm or may be incorrectly specified. Additionally, note that Vite 7.3.1 requires Node.js version ^20.19.0 or >=22.12.0, which is a very recent requirement that may not be available in all environments. Please verify this version exists and the Node.js version requirement is acceptable for the target deployment environment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
by the way - if wanted I'd be happy to do a similar treatment on the other example - lemme know! |
commit: |
|
recheck |
|
"recheck" was supposed to trigger the CLA assistant to check for signatures again but it failed. I'm not sure what's going on here. I'll just bypass it. |
|
Thanks!
I would say it's not a high priority but if you feel like working on it I'm happy to accept improved examples! |
I've been playing around with Cap'n Web and I was looking at this example and thought it might be welcome to give it a refresh. Feel free to review/read commit-by-commit (I left some discourse there, too).
dark mode, too
Notable changes: