fix(widget): emit shared SwiftUI FFI helpers once per bundle (#5069)#5119
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces ChangesShared Swift FFI runtime extraction
Sequence Diagram(s)sequenceDiagram
participant Driver as compile_for_ios/watchos_widget
participant Codegen as perry_codegen_swiftui
participant FS as Output Directory
loop for each widget
Driver->>Codegen: compile_widget(widget)
Codegen-->>FS: {Name}.swift + {Name}Glue.swift
end
Driver->>Driver: scan all widgets for provider_func_name or app_group
alt any widget has provider/app_group
Driver->>Codegen: emit_shared_runtime(first_app_group)
Codegen-->>Driver: PerryWidgetRuntime.swift source
Driver->>FS: write PerryWidgetRuntime.swift
Driver->>Driver: append to Swift compilation inputs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/perry/src/commands/compile/targets.rs (2)
297-302: ⚡ Quick winConsider validating consistent app group usage across widgets.
The current implementation uses the first widget's
app_groupfor the bundle-wide@_cdeclshared-storage bridge. If different widgets specify different app groups, they will all share the first widget's suite, which may cause unexpected shared-storage behavior.Consider adding a validation pass to ensure all widgets either:
- Use the same app group, or
- Don't specify an app group
This validation is out of scope for the current PR (which focuses on fixing the duplicate-symbol/inaccessibility regression), but would help catch configuration errors at compile time rather than runtime.
🤖 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 `@crates/perry/src/commands/compile/targets.rs` around lines 297 - 302, In the condition where you check if any widget has an app_group or provider_func_name, add a validation step after finding the app_group value via find_map to ensure all widgets that specify an app_group use the same one. Implement a check that iterates through all widgets and verifies either they all have no app_group, they all have the same app_group value, or raises a compilation error if inconsistencies are detected. This prevents the scenario where different widgets with different app_group values would silently use the first widget's app_group for the shared-storage bridge, which could cause unexpected runtime behavior.
564-569: ⚡ Quick winConsider validating consistent app group usage across widgets.
Same consideration as the iOS widget path (lines 297-302): multiple widgets with different app groups will all use the first widget's suite in the shared-storage bridge. Consider validation to catch configuration errors, though this is out of scope for the current PR.
🤖 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 `@crates/perry/src/commands/compile/targets.rs` around lines 564 - 569, Add validation to ensure consistent app_group usage across all widgets in the SwiftUI target path. Before calling emit_shared_runtime with the app_group derived from find_map, validate that all widgets which have an app_group specified use the same value. If inconsistencies are detected, either error out or log a warning to prevent silent misconfiguration where multiple widgets with different app groups would incorrectly use only the first widget's app group in the shared-storage bridge. This validation should mirror the consistency checking that should also be applied to the iOS widget path (lines 297-302).
🤖 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 `@crates/perry/src/commands/compile/targets.rs`:
- Around line 297-302: In the condition where you check if any widget has an
app_group or provider_func_name, add a validation step after finding the
app_group value via find_map to ensure all widgets that specify an app_group use
the same one. Implement a check that iterates through all widgets and verifies
either they all have no app_group, they all have the same app_group value, or
raises a compilation error if inconsistencies are detected. This prevents the
scenario where different widgets with different app_group values would silently
use the first widget's app_group for the shared-storage bridge, which could
cause unexpected runtime behavior.
- Around line 564-569: Add validation to ensure consistent app_group usage
across all widgets in the SwiftUI target path. Before calling
emit_shared_runtime with the app_group derived from find_map, validate that all
widgets which have an app_group specified use the same value. If inconsistencies
are detected, either error out or log a warning to prevent silent
misconfiguration where multiple widgets with different app groups would
incorrectly use only the first widget's app group in the shared-storage bridge.
This validation should mirror the consistency checking that should also be
applied to the iOS widget path (lines 297-302).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 706f4d64-98e1-408c-8bbf-b72db9f953ac
📒 Files selected for processing (3)
crates/perry-codegen-swiftui/src/emit.rscrates/perry-codegen-swiftui/src/lib.rscrates/perry/src/commands/compile/targets.rs
proggeramlug
pushed a commit
that referenced
this pull request
Jun 14, 2026
…g app groups Address CodeRabbit review on #5119: the per-bundle PerryWidgetRuntime.swift emission was duplicated verbatim in the ios-widget and watchos-widget drivers. Extract it into write_shared_widget_runtime(), and — since the @_cdecl shared-storage bridge can bind only one app group per bundle — warn (non-fatal) when widgets declare differing app groups instead of silently routing them all through the first widget's suite. https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/perry/src/commands/compile/targets.rs`:
- Around line 418-436: The conflicting app groups warning is guarded by an `if
matches!(format, OutputFormat::Text)` condition, which prevents the warning from
being emitted when using JSON output format. This silently hides important
information about shared storage routing discrepancies. Remove the
OutputFormat::Text guard so the warning logic is executed regardless of the
output format chosen by the user. The eprintln! call already ensures stdout
remains clean for JSON serialization, so the guard is unnecessary and only
suppresses critical warnings in JSON mode.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db1bd4bd-3dc4-4b13-b831-20ce17484f6b
📒 Files selected for processing (1)
crates/perry/src/commands/compile/targets.rs
proggeramlug
pushed a commit
that referenced
this pull request
Jun 14, 2026
Address CodeRabbit review on #5119: the warning used eprintln! (stderr), so the OutputFormat::Text guard only suppressed a genuine misconfiguration signal under --format json without protecting the JSON on stdout. Drop the guard (and the now -unused format parameter) so the warning surfaces regardless of output format. https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
A perry/widget bundle with ≥2 Widget({...}) declarations failed under
`swiftc *.swift`: the shared perry-runtime FFI helpers and the
@_cdecl("perry_widget_shared_storage_get") bridge were emitted once per
widget into each {Name}Glue.swift. Since a bundle compiles as one Swift
module, `private` made them inaccessible from the sibling {Name}.swift
call sites, and non-private produced N redeclarations across N glue files.
Emit the shared FFI block + @_cdecl bridge once per bundle, internal, into
PerryWidgetRuntime.swift (emit_shared_runtime). Per-widget glue keeps only
the widget-unique @_silgen_name provider import. The ios/watchos widget
drivers write the shared file once after the per-widget loop, gated on any
widget using a native provider or shared storage, with the bundle-wide app
group resolved from the first widget that configures one.
https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
…g app groups Address CodeRabbit review on #5119: the per-bundle PerryWidgetRuntime.swift emission was duplicated verbatim in the ios-widget and watchos-widget drivers. Extract it into write_shared_widget_runtime(), and — since the @_cdecl shared-storage bridge can bind only one app group per bundle — warn (non-fatal) when widgets declare differing app groups instead of silently routing them all through the first widget's suite. https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
proggeramlug
pushed a commit
that referenced
this pull request
Jun 14, 2026
Address CodeRabbit review on #5119: the warning used eprintln! (stderr), so the OutputFormat::Text guard only suppressed a genuine misconfiguration signal under --format json without protecting the JSON on stdout. Drop the guard (and the now -unused format parameter) so the warning surfaces regardless of output format. https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
proggeramlug
force-pushed
the
claude/issue-5069-v6k5t6
branch
from
1043adf to
431f60b
Compare
Address CodeRabbit review on #5119: the warning used eprintln! (stderr), so the OutputFormat::Text guard only suppressed a genuine misconfiguration signal under --format json without protecting the JSON on stdout. Drop the guard (and the now -unused format parameter) so the warning surfaces regardless of output format. https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
proggeramlug
force-pushed
the
claude/issue-5069-v6k5t6
branch
from
431f60b to
6ee593e
Compare
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
Fixes #5069. A
perry/widgetbundle with ≥2Widget({...})declarations failed to compile underswiftc *.swift.The shared perry-runtime FFI helpers (
perry_runtime_widget_init, the twoperry_nanbox_stringoverloads,perry_get_string_ptr/perry_get_string) plus the@_cdecl("perry_widget_shared_storage_get")bridge were emitted once per widget, into each{Name}Glue.swift. Since a widget bundle compiles as a single Swift module, neither per-file visibility worked:private(the TS-widget SwiftUI codegen produces Swift swiftc rejects: perry_nanbox_string redeclared, .utf8 unqualified — follow-up to #1179 #1294 fix) scoped each helper to its own glue file, but they're called from a sibling generated file ({Name}.swift) →'perry_get_string' is inaccessible due to 'private' protection level.private→ N copies across N glue files → redeclaration (the original TS-widget SwiftUI codegen produces Swift swiftc rejects: perry_nanbox_string redeclared, .utf8 unqualified — follow-up to #1179 #1294 failure).The
@_cdeclstorage bridge exports a fixed C symbol, so ≥2 widgets sharing an app group would also have duplicated it at link time.Fix
perry-codegen-swiftui/src/emit.rs— splitemit_glueintoemit_shared_runtime(app_group)(the FFI block +@_cdeclbridge, nowinternal) and a slimmedemit_gluethat keeps only the widget-unique@_silgen_nameprovider import.perry-codegen-swiftui/src/lib.rs—compile_widgetemits per-widget glue only whenprovider_func_nameis set; exposesemit_shared_runtime.perry/src/commands/compile/targets.rs— bothcompile_for_ios_widgetandcompile_for_watchos_widgetwritePerryWidgetRuntime.swiftonce after the per-widget loop, gated on any widget using a native provider or shared storage, with the bundle-wide app group resolved from the first widget that configures one.Tests
Added 3 unit tests covering the shared-runtime split (internal visibility, app-group storage bridge, per-widget glue carrying only the provider extern). Full crate suite (18 tests) passes;
cargo build --release -p perryis clean.Note
The downstream
.appexlink step from the issue (pulling inlibperry_runtime.a/ the TS provider objects) needs a macOS/Xcode toolchain to verify. This change unblocks theswiftccompile so that step is now reachable to confirm.https://claude.ai/code/session_01JJfeShA3WCh9HMiEBmqq39
Generated by Claude Code
Summary by CodeRabbit
Release Notes
New Features
PerryWidgetRuntime.swiftduring widget compilation, including shared runtime helpers and an app-group-based shared storage bridge when needed.Refactor
Tests