fix: expo plugin breaking due to override keyword needing to be public in version 54

[gladiuscode]

This PR fixes #93.

Summary by CodeRabbit

• Bug Fixes

  • Corrected SDK version parsing to use semantic version formatting and adjusted Swift method keyword generation based on major SDK version.

• Tests

  • Added coverage for SDK 54+ and improved snapshot assertions for better validation of SDK-specific behavior.

[coderabbitai]

Walkthrough

computeMethodPrefix now parses only the major SDK version (segment before the first dot) and returns no prefix for major 53, "public override" for majors > 53; tests were updated to use semantic version strings and a new SDK 54 test plus snapshot assertions were added.

Changes

Cohort / File(s)	Summary
Test suite updates plugin/__tests__/withRNOrientationAppDelegate.spec.ts	Updated test descriptions and inputs to use semantic versions ('52.0.0', '53.0.0', '54.0.0'); added a new test for SDK 54; added immediate snapshot assertion in the 53-series test; adjusted expectations to reflect "public override" behavior changes.
Override keyword logic plugin/src/withRNOrientationAppDelegate.ts	computeMethodPrefix changed to extract the major version (substring before first dot), validate it as a number, return "" for major === 53, return "public override" for major > 53, and handle malformed/missing versions by returning "". swiftFileUpdater signature unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant Test as Tests
  participant Updater as swiftFileUpdater
  participant Prefix as computeMethodPrefix
  Note over Test,Updater: Test supplies originalContents + sdkVersion
  Test->>Updater: call swiftFileUpdater(originalContents, sdkVersion)
  Updater->>Prefix: computeMethodPrefix(sdkVersion)
  alt sdkVersion missing or malformed
    Prefix-->>Updater: ""
  else major === 53
    Prefix-->>Updater: ""
  else major > 53
    Prefix-->>Updater: "public override"
  end
  Updater-->>Test: modified Swift file contents (with selected prefix)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay attention to edge-case handling in computeMethodPrefix (empty, non-numeric, or multi-segment versions).
• Verify test snapshots match the intended Swift method signatures for 52.x, 53.x, and 54.x.
• Confirm no unintended changes to exported function signatures.

Possibly related PRs

• fix(iOS): expo plugin override keyword on ios #86 — modifies the same swiftFileUpdater / override-keyword logic in plugin/src/withRNOrientationAppDelegate.ts.
• fix: expo plugin breaking due to override keyword not supported in version 54 #92 — adjusts computeMethodPrefix behavior and SDK-version-based prefixing logic.

Suggested labels

iOS

Poem

🐇 I hopped through versions, sniffed the code,
Found the major number on the road.
For fifty-four I shout, "public override!" 🥕
Tests snug in a semantic coat,
AppDelegate hops with pride.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "fix: expo plugin breaking due to override keyword needing to be public in version 54" directly summarizes the main change in the pull request. The code modifications in withRNOrientationAppDelegate.ts update the computeMethodPrefix function to return "public override" for SDK versions greater than 53, which resolves the compilation failure when loading the plugin in version 54. The test updates validate this behavior with semantic versioning inputs and appropriate snapshot assertions. The title is specific, clear, and accurately reflects the core fix being implemented.
Linked Issues Check	✅ Passed	The code changes directly address the requirements in issue #93 by fixing the computeMethodPrefix function to return "public override" for SDK versions greater than 53, which resolves the compilation failure when loading the plugin in version 54. The implementation correctly isolates the major version from semantic version strings and applies the appropriate override keyword based on version logic. The test changes include new test cases for version 54.0.0 with snapshot assertions, validating that the fix produces the expected "public override" behavior. All coding requirements from the linked issue are satisfied by these modifications.
Out of Scope Changes Check	✅ Passed	All changes in this pull request are directly related to fixing the issue described in #93. The modifications to computeMethodPrefix in withRNOrientationAppDelegate.ts implement the core fix to use "public override" for version 54+, and the test file updates validate this behavior with appropriate semantic versioning inputs and snapshot assertions. There are no extraneous changes, refactoring, or modifications unrelated to the bug fix objective. All code changes are appropriately scoped to address the compilation failure issue.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 93-bug-expo-plugin-breaking-due-to-override-keyword-needing-to-be-public-in-version-54

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2844a4 and 4247375.

⛔ Files ignored due to path filters (1)

• plugin/__tests__/__snapshots__/withRNOrientationAppDelegate.spec.ts.snap is excluded by !**/*.snap

📒 Files selected for processing (2)

• plugin/__tests__/withRNOrientationAppDelegate.spec.ts (1 hunks)
• plugin/src/withRNOrientationAppDelegate.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

plugin/__tests__/withRNOrientationAppDelegate.spec.ts (3)

plugin/src/withRNOrientationAppDelegate.ts (1)

• swiftFileUpdater (58-102)

plugin/__tests__/fixtures/AppDelegate52.swift (1)

• AppDelegate (6-30)

plugin/__tests__/fixtures/AppDelegate53.swift (1)

• AppDelegate (5-53)

🔇 Additional comments (4)

plugin/src/withRNOrientationAppDelegate.ts (1)

81-101: LGTM! The implementation correctly addresses issue #93.

The version parsing logic properly extracts the major version and applies the correct prefix:

• SDK 53: No override (empty string)
• SDK 52, 54+: "public override"

The guards for missing, empty, or invalid SDK versions are appropriate, and the semantic version parsing using split('.').at(0) correctly handles versions like "54.0.0".

plugin/__tests__/withRNOrientationAppDelegate.spec.ts (3)

22-31: LGTM! Test correctly validates SDK 52 behavior.

The test description accurately reflects that SDK 52 (and below) should have the "public override" prefix, and the semantic version format "52.0.0" is appropriate.

---

33-42: LGTM! Test correctly validates SDK 53 behavior and past issue resolved.

The test description now accurately states that SDK 53 should be "without public override", matching the implementation. The past review comment about the incorrect description has been addressed.

---

44-53: LGTM! Test correctly validates SDK 54 behavior and directly addresses issue #93.

The test description now accurately reflects that SDK 54 (and above) should have "public override", which is the core fix for the bug reported in issue #93. The reuse of the AppDelegate53.swift fixture is appropriate since the test validates the string manipulation logic. The past review comment about the incorrect description has been addressed.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83d7f3f and a2844a4.

⛔ Files ignored due to path filters (1)

• plugin/__tests__/__snapshots__/withRNOrientationAppDelegate.spec.ts.snap is excluded by !**/*.snap

📒 Files selected for processing (2)

• plugin/__tests__/withRNOrientationAppDelegate.spec.ts (1 hunks)
• plugin/src/withRNOrientationAppDelegate.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

plugin/__tests__/withRNOrientationAppDelegate.spec.ts (3)

plugin/src/withRNOrientationAppDelegate.ts (1)

• swiftFileUpdater (58-102)

plugin/__tests__/fixtures/AppDelegate52.swift (2)

• AppDelegate (6-30)
• application (8-17)

plugin/__tests__/fixtures/AppDelegate53.swift (1)

• AppDelegate (5-53)

🔇 Additional comments (1)

plugin/src/withRNOrientationAppDelegate.ts (1)

86-101: The implementation is correct; test case names are misleading but snapshots reflect the correct behavior.

The snapshot verification reveals the original review comment misinterpreted the test case names. While the descriptions are backwards:

• SDK 52 test says "having public override" but actually shows NO override (line 100: func application(...))
• SDK 53+ tests say "without override" but actually show "public override" (lines 157, 238: public override func application(...))

The actual snapshot content is correct and matches the implementation logic:

• SDK < 53: empty prefix (no override keyword)
• SDK ≥ 53: "public override" prefix

This correctly addresses the original issue #93 where SDK 54 requires the override keyword.

Likely an incorrect or invalid review comment.