feat(react): expose ET data processors#2646
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughExtracts duplicated processData logic into createProcessData, adds tests, re-exports related types and a deprecated root API, and updates two Lynx env modules to use the shared factory. ChangesData processor factory extraction and refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
React External#1475 Bundle Size — 695.4KiB (+0.01%).97e55aa(current) vs 353b1b7 main#1415(baseline) Bundle metrics
|
| Current #1475 |
Baseline #1415 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
41% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 regression
| Current #1475 |
Baseline #1415 |
|
|---|---|---|
| Other |
695.4KiB (+0.01%) |
695.33KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#629 Bundle Size — 200.08KiB (+0.06%).97e55aa(current) vs 353b1b7 main#569(baseline) Bundle metrics
|
| Current #629 |
Baseline #569 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
27.1% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
89(-2.2%) |
91 |
| Duplicate Modules |
27 |
27 |
| Duplicate Code |
39.81%(-0.55%) |
40.03% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #629 |
Baseline #569 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
54.32KiB (+0.23%) |
54.19KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1493 Bundle Size — 208.12KiB (+0.01%).97e55aa(current) vs 353b1b7 main#1434(baseline) Bundle metrics
|
| Current #1493 |
Baseline #1434 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
46.55% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
193(+0.52%) |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.24%(-0.36%) |
44.4% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #1493 |
Baseline #1434 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.89KiB (+0.02%) |
96.86KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8361 Bundle Size — 237.17KiB (~+0.01%).97e55aa(current) vs 353b1b7 main#8301(baseline) Bundle metrics
|
| Current #8361 |
Baseline #8301 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
38.54% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
198(+0.51%) |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.73%(-0.36%) |
44.89% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #8361 |
Baseline #8301 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.42KiB (+0.02%) |
91.39KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9934 Bundle Size — 901.35KiB (0%).97e55aa(current) vs 353b1b7 main#9874(baseline) Bundle metrics
|
| Current #9934 |
Baseline #9874 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
229 |
229 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.22% |
27.22% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9934 |
Baseline #9874 |
|
|---|---|---|
| JS |
497.08KiB |
497.08KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
Merging this PR will not alter performance
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | 002-hello-reactLynx-destroyBackground |
915.9 µs | 865.2 µs | +5.86% |
| ❌ | 008-many-use-state-destroyBackground |
8 ms | 9.5 ms | -15.93% |
| ⚡ | transform 1000 view elements |
44.7 ms | 40 ms | +11.69% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Yradex:parallel/element-template/02-register-data-processors (97e55aa) with main (9449860)2
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(6f0042f) during the generation of this report, so 9449860 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Yradex
force-pushed
the
parallel/element-template/02-register-data-processors
branch
from
ce2119e to
26d844a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/runtime/src/core/lynx-data-processors.ts (1)
13-31: 💤 Low valueUse
try/finallysoprofileEndis always called.
profileStart('processData')is paired with aprofileEnd()placed after the catch block. Iflynx.reportError(or any other code in the catch) throws,profileEndis skipped and the profile span leaks for the lifetime of the runtime. Wrapping intry/finallykeeps the pair symmetric without changing observable behavior on the happy path.♻️ Proposed fix
return (data, processorName) => { - if (typeof __PROFILE__ !== 'undefined' && __PROFILE__) { - profileStart('processData'); - } - - let result: InitData | InitDataRaw; - try { - if (processorName) { - result = dataProcessorDefinition?.dataProcessors?.[processorName]?.(data) as InitData ?? data; - } else { - result = dataProcessorDefinition?.defaultDataProcessor?.(data) ?? data; - } - } catch (error: unknown) { - lynx.reportError(error as Error); - result = {}; - } - - if (typeof __PROFILE__ !== 'undefined' && __PROFILE__) { - profileEnd(); - } + const profileEnabled = typeof __PROFILE__ !== 'undefined' && __PROFILE__; + if (profileEnabled) { + profileStart('processData'); + } + + let result: InitData | InitDataRaw; + try { + try { + if (processorName) { + result = dataProcessorDefinition?.dataProcessors?.[processorName]?.(data) as InitData ?? data; + } else { + result = dataProcessorDefinition?.defaultDataProcessor?.(data) ?? data; + } + } catch (error: unknown) { + lynx.reportError(error as Error); + result = {}; + } + } finally { + if (profileEnabled) { + profileEnd(); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/runtime/src/core/lynx-data-processors.ts` around lines 13 - 31, The profiling span started by profileStart('processData') can leak if an exception is thrown in the catch block; refactor the block so profileStart is followed by a try { ... } finally { profileEnd(); } and move the existing data processing/exception handling inside that try: call the processor (using processorName and dataProcessorDefinition to compute result), catch errors with lynx.reportError(error) and set result = {}, then ensure profileEnd() is invoked in the finally regardless of errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/react/runtime/src/core/lynx-data-processors.ts`:
- Around line 13-31: The profiling span started by profileStart('processData')
can leak if an exception is thrown in the catch block; refactor the block so
profileStart is followed by a try { ... } finally { profileEnd(); } and move the
existing data processing/exception handling inside that try: call the processor
(using processorName and dataProcessorDefinition to compute result), catch
errors with lynx.reportError(error) and set result = {}, then ensure
profileEnd() is invoked in the finally regardless of errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0cb5e5cc-06cf-45a2-92e3-29a0bf062d75
📒 Files selected for processing (9)
packages/react/runtime/__test__/core/lynx-data-processors.test.tspackages/react/runtime/__test__/element-template/imports/register-data-processors.test.tspackages/react/runtime/__test__/element-template/vitest.config.tspackages/react/runtime/__test__/snapshot/lynx-data-processors.test.tspackages/react/runtime/src/core/lynx-data-processors.tspackages/react/runtime/src/element-template/client/root.tspackages/react/runtime/src/element-template/index.tspackages/react/runtime/src/element-template/lynx/env.tspackages/react/runtime/src/snapshot/lynx/env.ts
✅ Files skipped from review due to trivial changes (3)
- packages/react/runtime/test/snapshot/lynx-data-processors.test.ts
- packages/react/runtime/test/element-template/vitest.config.ts
- packages/react/runtime/src/element-template/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/react/runtime/src/element-template/client/root.ts
- packages/react/runtime/test/element-template/imports/register-data-processors.test.ts
- packages/react/runtime/src/snapshot/lynx/env.ts
- packages/react/runtime/test/core/lynx-data-processors.test.ts
- packages/react/runtime/src/element-template/lynx/env.ts
`Vitest (Windows)` failed on
`packages/rspeedy/plugin-react/test/background-only.test.ts:153`:
AssertionError: expected error.message === 'Rspack build failed.'
Expected: "Rspack build failed."
Received: "ENOENT: no such file or directory,
open 'C:\\…\\test\\dist\\.rspeedy\\rspeedy.config.js'"
The test expects rsbuild to report `Rspack build failed.` when the
configured entry is missing, but on Windows the failure surfaces
earlier as an ENOENT trying to open a generated config file under the
test's dist tree. This PR doesn't touch
`packages/rspeedy/plugin-react`; Vitest (Ubuntu) passed; the same
Vitest (Windows) check passes on other open PRs against the same
upstream (lynx-family#2645, lynx-family#2646, lynx-family#2648). Windows-only rspeedy infrastructure
flake — same family as the `validate is not a function` race we hit
earlier. Empty commit to retry.
Yradex
force-pushed
the
parallel/element-template/02-register-data-processors
branch
from
26d844a to
97e55aa
Compare
2 participants
Summary by CodeRabbit
Tests
Refactor
Overview
root.registerDataProcessorsfrom the Element Template entry so ET roots can register InitData processors through the same Lynx environment hook used by Snapshot.Key Points
lynx.registerDataProcessorscall path for Snapshot and ET.@lynx-js/react/element-templateusers can callroot.registerDataProcessors(...).Checklist