-
Notifications
You must be signed in to change notification settings - Fork 16
build: Migrate from Webpack to Vite (closes #146, resolves #238). #175
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
Changes from all commits
49ab69f
35aabf2
b1a7f46
44ddeb4
98687b6
dcdc4b1
2d7b02a
d5e189c
221074d
48aed37
83c57c2
a7269a8
ff935ad
2097bbc
7edd320
cb37747
9124e3d
da2ecd7
a46a36b
95df1e0
139d161
c8184c8
4e7c916
bb7a380
1bae57d
cd5c0bf
ce20db7
9d4651e
011f7fa
866c5e8
26280ba
a866b3f
a751770
0bd761f
8d92be9
f234101
32c3ad3
664e02f
042fc29
08be042
e891f11
1cc0430
928739d
4dc9612
494172b
a6b156e
b05ed86
15b7db2
d030fb1
bfbd033
0cbd976
2b4d222
9624822
778f4c4
8d8b7e1
ee1483a
3c1660f
e2c1387
f2aba14
8597850
fa9090c
cb015ec
abdec7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,8 +3,31 @@ | |||||||||||||||||||||
This project adheres to YScope's [contribution guidelines][yscope-guidelines] as well as the | ||||||||||||||||||||||
project-specific guidelines below. | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Web workers | ||||||||||||||||||||||
|
||||||||||||||||||||||
## Importing web workers | ||||||||||||||||||||||
|
||||||||||||||||||||||
When importing web worker files, use Vite's `?worker` query suffix syntax: | ||||||||||||||||||||||
|
||||||||||||||||||||||
```ts | ||||||||||||||||||||||
import MainWorker from "../services/MainWorker.worker?worker"; | ||||||||||||||||||||||
|
||||||||||||||||||||||
const worker = new MainWorker(); | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
|
||||||||||||||||||||||
This special syntax tells Vite to transform the import as a web worker constructor. See | ||||||||||||||||||||||
[Vite's web worker documentation][vite-worker-query-suffix] for more details. | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Link to specific section in Vite documentation Consider linking directly to the specific section in Vite's documentation about workers for easier reference. - [Vite's web worker documentation][vite-worker-query-suffix] for more details.
+ [Vite's web worker documentation][vite-worker-query-suffix#web-workers] for more details. Then update the link reference: - [vite-worker-query-suffix]: https://vite.dev/guide/features.html#import-with-query-suffixes
+ [vite-worker-query-suffix]: https://vitejs.dev/guide/features.html#web-workers 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
# Naming | ||||||||||||||||||||||
|
||||||||||||||||||||||
## Web worker files | ||||||||||||||||||||||
|
||||||||||||||||||||||
Name web worker files with the extension, `.worker.ts`. This is to: | ||||||||||||||||||||||
|
||||||||||||||||||||||
* follow standard practices. | ||||||||||||||||||||||
* allow [eslint.config.mjs][eslint-config-mjs] to ignore `.worker.ts` files, suppressing | ||||||||||||||||||||||
`eslint-plugin-import:import/default` errors caused by Vite's `?worker` import syntax. | ||||||||||||||||||||||
Comment on lines
+25
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Refine bullet list for clarity and grammar The phrasing can be tightened. For example: - Name web worker files with the extension, `.worker.ts`. This is to:
+ Name web worker files with the extension `.worker.ts` to:
- * follow standard practices.
+ * follow standard practices
- * allow [eslint.config.mjs][eslint-config-mjs] to ignore `.worker.ts` files, suppressing
- `eslint-plugin-import:import/default` errors caused by Vite's `?worker` import syntax.
+ * allow [eslint.config.mjs][eslint-config-mjs] to ignore `.worker.ts` files and suppress
+ `eslint-plugin-import:import/default` errors caused by Vite’s `?worker` import syntax 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
## Index variables | ||||||||||||||||||||||
|
||||||||||||||||||||||
To differentiate variables that use different starting indexes (0 vs. 1), use the following naming | ||||||||||||||||||||||
|
@@ -30,4 +53,6 @@ To avoid including a state variable in a React Hook's dependency array, you can | |||||||||||||||||||||
the state variable with an additional `Ref` suffix. E.g., `logEventNumRef` is the reference variable | ||||||||||||||||||||||
that corresponds to the `logEventNum` state variable. | ||||||||||||||||||||||
|
||||||||||||||||||||||
[eslint-config-mjs]: https://github.com/y-scope/yscope-log-viewer/blob/main/eslint.config.mjs | ||||||||||||||||||||||
[vite-worker-query-suffix]: https://vite.dev/guide/features.html#import-with-query-suffixes | ||||||||||||||||||||||
[yscope-guidelines]: https://docs.yscope.com/dev-guide/contrib-guides-overview.html |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,26 @@ | ||||||
# Optimization guide | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Nitpick: Consistent title casing |
||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider cross-linking related guides +# See the [Building & Getting Started](../building-getting-started.md) guide for setup instructions. 📝 Committable suggestion
Suggested change
|
||||||
This doc outlines strategies and tools used to optimize the build's size and load time. | ||||||
|
||||||
Comment on lines
+3
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Enhance navigation with cross-links and refine wording
-This doc outlines strategies and tools used to optimize build size and load time.
+# This doc outlines strategies and tools used to optimize build size and load time.
+
+# See the [Building & Getting Started](../building-getting-started.md) guide for setup instructions.
🤖 Prompt for AI Agents
|
||||||
## Bundle analysis | ||||||
|
||||||
To generate a bundle analysis report, run: | ||||||
|
||||||
```shell | ||||||
# You may be prompted to install `vite-bundle-visualizer`. Accept the prompt to install it. | ||||||
npm run analyze:size | ||||||
``` | ||||||
|
||||||
This will use [`vite-bundle-visualizer`][vite-bundle-visualizer] to generate an interactive visual | ||||||
breakdown of bundle contents, helping identify large dependencies and optimization opportunities. | ||||||
|
||||||
## Future strategies | ||||||
|
||||||
The following optimization strategies are planned for the future: | ||||||
|
||||||
* **Code splitting**: We can split the code into smaller chunks to improve load time, especially | ||||||
when using lazy loading. | ||||||
* **Lazy loading**: We can load components or modules only when they are needed. This can be | ||||||
achieved by using dynamic imports or React's `lazy()` and `Suspense` features. | ||||||
|
||||||
Comment on lines
+21
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Optional: Enrich future strategies with Vite specifics 🤖 Prompt for AI Agents
|
||||||
[vite-bundle-visualizer]: https://www.npmjs.com/package/vite-bundle-visualizer |
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.
🧹 Nitpick (assertive)
Standardize capitalization of "web workers"
Be consistent with capitalization. Past discussions indicated "web workers" should not be treated as a proper noun. Use lowercase "web workers" throughout the document.
Also applies to: 10-10, 18-18
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Multiple top-level headings in the same document
null
(MD025, single-title, single-h1)