Image component boosting proposal#66
Open
adamivancza wants to merge 9 commits into
Open
Conversation
added 8 commits
June 25, 2026 16:33
|
Someone is attempting to deploy a commit to the Kuatsu Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds a new "Image" optimizer to react-native-boost, transforming eligible ChangesImage Optimizer Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant NativeImageComponent as native-image.tsx
participant RNPlatform as Platform
participant RuntimeIndex as runtime/index.ts
participant Consumer as Optimized Component
Consumer->>NativeImageComponent: import NativeImage
NativeImageComponent->>RNPlatform: check Platform.OS
alt web
NativeImageComponent-->>Consumer: return RN Image
else native
NativeImageComponent->>NativeImageComponent: require internal host
NativeImageComponent-->>Consumer: return default or fallback Image
end
Consumer->>RuntimeIndex: processImageSourceProps(props)
RuntimeIndex-->>Consumer: normalized source/style/resizeMode
Consumer->>RuntimeIndex: processImageAccessibilityProps(props)
RuntimeIndex-->>Consumer: normalized accessibility props
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/react-native-boost/src/plugin/__tests__/parity/fuzz/vocabulary.ts (1)
343-348: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
crossOrigin/referrerPolicynever fuzz null/undefined.Both request-header-driving props use plain
fc.constantFrom(...)while sibling props likeresizeMode/tintColorin the same vocab wrap values withwithNullish(...). Since the PR explicitly adds "request headers derived fromcrossOriginandreferrerPolicy" handling, the fuzz suite won't exercise the nullish-value code path for these two props, leaving that branch of the request-header logic unverified by fuzzing.♻️ Proposed fix
- { name: 'crossOrigin', arb: fc.constantFrom('"anonymous"', '"use-credentials"'), disposition: 'request headers' }, + { name: 'crossOrigin', arb: withNullish(fc.constantFrom('"anonymous"', '"use-credentials"')), disposition: 'request headers' }, { name: 'referrerPolicy', - arb: fc.constantFrom('"origin"', '"no-referrer"', '"same-origin"'), + arb: withNullish(fc.constantFrom('"origin"', '"no-referrer"', '"same-origin"')), disposition: 'request headers', },🤖 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-native-boost/src/plugin/__tests__/parity/fuzz/vocabulary.ts` around lines 343 - 348, The fuzz vocabulary for the request-header props is missing nullish coverage for crossOrigin and referrerPolicy, so the new request-header derivation path is not being exercised with null/undefined inputs. Update the vocabulary entries in the vocabulary test fixture to match sibling props like resizeMode and tintColor by wrapping the existing fc.constantFrom values with withNullish, keeping the same symbols crossOrigin and referrerPolicy so the nullish branch in the request-header logic is fuzzed too.packages/react-native-boost/src/plugin/__tests__/native-attribute-conformance.test.ts (1)
204-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
it.eachskip pattern can mask coverage regressions for passthrough props.
if (!result?.optimized) return;(Line 210) causes the test to pass vacuously whenever the optimizer bails out — appropriate forIMAGE_WRAPPER_ONLY_PROPS(bail-out is expected/acceptable there), but forIMAGE_PASSTHROUGH_PROPSa regression that makes the optimizer unexpectedly bail on a legitimate native prop combination would silently pass this suite instead of failing. Only the single base-case assertion at Line 174 currently guarantees "exercises the optimized path" coverage.Consider asserting
result?.optimized === truefor entries sourced fromIMAGE_PASSTHROUGH_PROPSspecifically (splitting theit.eachor tagging entries), so a bail-out regression on a legitimate native prop is caught.♻️ Proposed refactor to assert optimization for passthrough props
describe('Image', () => { - it.each([...IMAGE_WRAPPER_ONLY_PROPS, ...IMAGE_PASSTHROUGH_PROPS])( - 'leaves only native attributes on the host for <Image %s />', - (attributes) => { - const result = optimizeAndInspect(imageSource(attributes), imageOptimizer, 'Image', 'ios'); - if (!result?.optimized) return; // bailed out: nothing reaches the native component - const leaked = result.attributes.filter((attribute) => !NATIVE_IMAGE_ATTRIBUTES.has(attribute)); - expect(leaked, `optimized <Image ${attributes} /> leaks non-native attribute(s): ${leaked.join(', ')}`).toEqual( - [] - ); - } - ); + it.each(IMAGE_WRAPPER_ONLY_PROPS)( + 'leaves only native attributes on the host for <Image %s /> (if optimized)', + (attributes) => { + const result = optimizeAndInspect(imageSource(attributes), imageOptimizer, 'Image', 'ios'); + if (!result?.optimized) return; // bailed out: nothing reaches the native component + const leaked = result.attributes.filter((attribute) => !NATIVE_IMAGE_ATTRIBUTES.has(attribute)); + expect(leaked, `optimized <Image ${attributes} /> leaks non-native attribute(s): ${leaked.join(', ')}`).toEqual( + [] + ); + } + ); + + it.each(IMAGE_PASSTHROUGH_PROPS)( + 'optimizes and leaves only native attributes on the host for <Image %s />', + (attributes) => { + const result = optimizeAndInspect(imageSource(attributes), imageOptimizer, 'Image', 'ios'); + expect(result?.optimized, `<Image ${attributes} /> unexpectedly bailed out`).toBe(true); + const leaked = result!.attributes.filter((attribute) => !NATIVE_IMAGE_ATTRIBUTES.has(attribute)); + expect(leaked, `optimized <Image ${attributes} /> leaks non-native attribute(s): ${leaked.join(', ')}`).toEqual( + [] + ); + } + ); });🤖 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-native-boost/src/plugin/__tests__/native-attribute-conformance.test.ts` around lines 204 - 217, The Image native-attribute conformance test is skipping on optimizer bailouts in a way that can hide regressions for passthrough props. Update the `describe('Image')` / `it.each([...IMAGE_WRAPPER_ONLY_PROPS, ...IMAGE_PASSTHROUGH_PROPS])` test so `IMAGE_PASSTHROUGH_PROPS` cases explicitly require `optimizeAndInspect(...)` to return an optimized result instead of returning early, while keeping the bail-out allowance only for wrapper-only props. Split the table or tag entries so the `result?.optimized` expectation is enforced for passthrough combinations and the leak check still uses `NATIVE_IMAGE_ATTRIBUTES`.packages/react-native-boost/src/runtime/components/native-image.test.ts (1)
33-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing happy-path test for successful native host resolution.
Both tests cover fallback scenarios (load failure, web short-circuit) but neither verifies that
NativeImageresolves to the actual native host component when loading succeeds on a non-web platform — the primary intended behavior ofresolveNativeImageComponent.✅ Suggested additional test
+ it('resolves to the internal native host when loading succeeds on a non-web platform', async () => { + const { NativeImage } = await importNativeImage({ os: 'android' }); + + expect(NativeImage).toBe(nativeHost); + });🤖 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-native-boost/src/runtime/components/native-image.test.ts` around lines 33 - 52, Add a happy-path test in NativeImage that verifies resolveNativeImageComponent returns the loaded native host when loadNativeComponent succeeds on a non-web platform. Reuse the existing NativeImage describe block and the importNativeImage helper, but mock loadNativeComponent to return the nativeHost default export without throwing, then assert NativeImage is the nativeHost component and not reactNativeImage. Keep the existing fallback tests unchanged.
🤖 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.
Inline comments:
In `@packages/react-native-boost/src/plugin/__tests__/parity/normalize.ts`:
- Around line 30-69: The normalizeImage helper is deleting the very props that
the image parity cases are intended to verify. Update normalizeImage in the
parity normalize test to stop stripping width, height, crossOrigin,
referrerPolicy, alt, and the aria-* fields before the toEqual comparison, or
move those checks into explicit assertions so IMAGE_CASES can validate
request-header synthesis, ARIA normalization, and sizing behavior through
normalizeImage.
---
Nitpick comments:
In
`@packages/react-native-boost/src/plugin/__tests__/native-attribute-conformance.test.ts`:
- Around line 204-217: The Image native-attribute conformance test is skipping
on optimizer bailouts in a way that can hide regressions for passthrough props.
Update the `describe('Image')` / `it.each([...IMAGE_WRAPPER_ONLY_PROPS,
...IMAGE_PASSTHROUGH_PROPS])` test so `IMAGE_PASSTHROUGH_PROPS` cases explicitly
require `optimizeAndInspect(...)` to return an optimized result instead of
returning early, while keeping the bail-out allowance only for wrapper-only
props. Split the table or tag entries so the `result?.optimized` expectation is
enforced for passthrough combinations and the leak check still uses
`NATIVE_IMAGE_ATTRIBUTES`.
In `@packages/react-native-boost/src/plugin/__tests__/parity/fuzz/vocabulary.ts`:
- Around line 343-348: The fuzz vocabulary for the request-header props is
missing nullish coverage for crossOrigin and referrerPolicy, so the new
request-header derivation path is not being exercised with null/undefined
inputs. Update the vocabulary entries in the vocabulary test fixture to match
sibling props like resizeMode and tintColor by wrapping the existing
fc.constantFrom values with withNullish, keeping the same symbols crossOrigin
and referrerPolicy so the nullish branch in the request-header logic is fuzzed
too.
In `@packages/react-native-boost/src/runtime/components/native-image.test.ts`:
- Around line 33-52: Add a happy-path test in NativeImage that verifies
resolveNativeImageComponent returns the loaded native host when
loadNativeComponent succeeds on a non-web platform. Reuse the existing
NativeImage describe block and the importNativeImage helper, but mock
loadNativeComponent to return the nativeHost default export without throwing,
then assert NativeImage is the nativeHost component and not reactNativeImage.
Keep the existing fallback tests unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1834b295-5be9-43f5-81f1-df521c44e66f
📒 Files selected for processing (71)
apps/example/src/screens/benchmark.tsxpackages/react-native-boost/src/plugin/__tests__/native-attribute-conformance.test.tspackages/react-native-boost/src/plugin/__tests__/native-valid-attributes.tspackages/react-native-boost/src/plugin/__tests__/parity/boost.tspackages/react-native-boost/src/plugin/__tests__/parity/capture.tsxpackages/react-native-boost/src/plugin/__tests__/parity/fuzz/fuzz.test.tspackages/react-native-boost/src/plugin/__tests__/parity/fuzz/generator.tspackages/react-native-boost/src/plugin/__tests__/parity/fuzz/vocabulary.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/ImageViewNativeComponent.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/NativeImageLoader.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/ReactNativeFeatureFlags.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/StyleSheet.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/TextInlineImageNativeComponent.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/boost-runtime.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/react-native.tspackages/react-native-boost/src/plugin/__tests__/parity/mocks/resolveAssetSource.tspackages/react-native-boost/src/plugin/__tests__/parity/normalize.tspackages/react-native-boost/src/plugin/__tests__/parity/parity.test.tspackages/react-native-boost/src/plugin/__tests__/parity/vitest.config.parity.mtspackages/react-native-boost/src/plugin/__tests__/parity/wrapper.tspackages/react-native-boost/src/plugin/index.tspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/accessibility-props/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/accessibility-props/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/accessibility-spread-bails/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/accessibility-spread-bails/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/basic/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/basic/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/dynamic-request-headers-runtime/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/dynamic-request-headers-runtime/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/dynamic-source-runtime/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/dynamic-source-runtime/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/event-bails/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/event-bails/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/fallback-semantics/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/fallback-semantics/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/force-dynamic-runtime/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/force-dynamic-runtime/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/native-props/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/native-props/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/request-headers/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/request-headers/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/require-source/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/require-source/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/source-array/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/source-array/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/spread-wrapper-props-bails/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/spread-wrapper-props-bails/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/src-precedence/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/src-precedence/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/src-prop/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/src-prop/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/static-style-derived-props/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/static-style-derived-props/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/text-ancestor-bails/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/text-ancestor-bails/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/view-props/code.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/fixtures/view-props/output.jspackages/react-native-boost/src/plugin/optimizers/image/__tests__/index.test.tspackages/react-native-boost/src/plugin/optimizers/image/index.tspackages/react-native-boost/src/plugin/types/index.tspackages/react-native-boost/src/plugin/utils/common/base.tspackages/react-native-boost/src/plugin/utils/generate-test-plugin.tspackages/react-native-boost/src/runtime/__tests__/index.test.tspackages/react-native-boost/src/runtime/__tests__/mocks/ImageViewNativeComponent.tspackages/react-native-boost/src/runtime/__tests__/mocks/react-native.tspackages/react-native-boost/src/runtime/components/native-image.test.tspackages/react-native-boost/src/runtime/components/native-image.tsxpackages/react-native-boost/src/runtime/index.tspackages/react-native-boost/src/runtime/index.web.tspackages/react-native-boost/src/runtime/types/react-native.d.tspackages/react-native-boost/vitest.config.ts
Contributor
|
Thanks for your work on this! I'll have a look at it over the weekend. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a boosted
Imageoptimizer that rewrites safereact-nativeImageusages toNativeImage.Includes support for:
source/srcsource={require(...)}crossOrigin/referrerPolicyValidation
Summary by CodeRabbit
New Features
Imageelements, including handling of static and dynamic sources, accessibility props, and image-specific styles.Bug Fixes