ci(benchmark/react): check Codspeed before use it#1720
Conversation
|
📝 WalkthroughWalkthroughAdds runtime guards around Codspeed instrumentation in benchmark/react/plugins/pluginScriptLoad.mjs. startBenchmark, stopBenchmark, and setExecutedBenchmark now execute only if Codspeed is defined. runAfterLoadScript.flush() remains unguarded. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
benchmark/react/plugins/pluginScriptLoad.mjs (2)
27-29: Harden the guard to avoid TypeErrors when Codspeed is present but APIs are missing.Also verify the method exists, not just the object.
-if (typeof Codspeed !== "undefined") { - Codspeed.startBenchmark(); -} +if (typeof Codspeed !== "undefined" && typeof Codspeed.startBenchmark === "function") { + Codspeed.startBenchmark(); +}
45-48: Simplify executed-benchmark string at build-time and guard individual APIs.Avoid nested runtime templates and check each Codspeed API before calling.
-if (typeof Codspeed !== "undefined") { - Codspeed.stopBenchmark(); - Codspeed.setExecutedBenchmark(`${caseDir}::\${${JSON.stringify(chunkName)}}_LoadScript`); -} +if (typeof Codspeed !== "undefined") { + if (typeof Codspeed.stopBenchmark === "function") Codspeed.stopBenchmark(); + if (typeof Codspeed.setExecutedBenchmark === "function") { + Codspeed.setExecutedBenchmark( + ${JSON.stringify(`${caseDir}::${chunkName}_LoadScript`)} + ); + } +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
benchmark/react/plugins/pluginScriptLoad.mjs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1720 will not alter performanceComparing 🎉 Hooray!
|
Web Explorer#5101 Bundle Size — 362.79KiB (0%).d59579b(current) vs 2800dce main#5090(baseline) Bundle metrics
|
| Current #5101 |
Baseline #5090 |
|
|---|---|---|
 Initial JS |
145.21KiB |
145.21KiB |
| Initial CSS |
31.89KiB |
31.89KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
219 |
219 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.33% |
3.33% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5101 |
Baseline #5090 |
|
|---|---|---|
| JS |
236.88KiB |
236.88KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
31.89KiB |
31.89KiB |
Bundle analysis report Branch hzy:p/hzy/bench_12 Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#5108 Bundle Size — 238.2KiB (0%).d59579b(current) vs 2800dce main#5097(baseline) Bundle metrics
|
| Current #5108 |
Baseline #5097 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
163 |
163 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
46.88% |
46.88% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5108 |
Baseline #5097 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
92.45KiB |
92.45KiB |
Bundle analysis report Branch hzy:p/hzy/bench_12 Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
benchmark/react/plugins/pluginScriptLoad.mjs (2)
27-29: Guard is good; consider using globalThis + optional chaining to avoid null/shape edge cases.Covers the ReferenceError. Small hardening: if a build sets
Codspeed = nullor exposes a partial API, optional chaining avoids crashes and keeps intent clear.-if (typeof Codspeed !== "undefined") { - Codspeed.startBenchmark(); -} +// Safe for ESM and handles null/partial API +globalThis.Codspeed?.startBenchmark?.();
45-48: Simplify banner string and harden calls; avoid nested${…}templating.The nested interpolation works but is brittle and harder to read. Inject the fully-resolved name at build time, and use
globalThis+ optional chaining to tolerate partial Codspeed APIs.-if (typeof Codspeed !== "undefined") { - Codspeed.stopBenchmark(); - Codspeed.setExecutedBenchmark(`${caseDir}::${${JSON.stringify(chunkName)}}_LoadScript`); -} +if (globalThis.Codspeed) { + globalThis.Codspeed.stopBenchmark?.(); + // Build-time string injection for clarity and safety + globalThis.Codspeed.setExecutedBenchmark?.( + ${JSON.stringify(`${caseDir}::${chunkName}_LoadScript`)} + ); +}If you adopt this style here, consider mirroring it for the
startBenchmarkblock above for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
benchmark/react/plugins/pluginScriptLoad.mjs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: test-rust / rustfmt
2 participants
Summary by CodeRabbit
Checklist