Fixed react-world-flags interop so shade's Flag renders under both bundlers#29137
Fixed react-world-flags interop so shade's Flag renders under both bundlers#291379larsons wants to merge 1 commit into
Conversation
…ndlers ref https://linear.app/ghost/issue/PLA-96/spike-rolldown-vite-drop-in-build-benchmarks react-world-flags is a CommonJS module shaped `{ __esModule: true, default: Component }`. esbuild/Rollup unwrap the default automatically, but Rolldown returns the wrapper object, so `<ReactFlag/>` renders an object and crashes the analytics location/source views. This adds a bundler-agnostic interopDefault() unwrap so the component resolves under both toolchains. Behavior-preserving under the current Rollup build; unblocks the Rolldown evaluation without changing anything for Rollup.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 32s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run-many -t test:unit -p @tryghost/shade,@tr... |
✅ Succeeded | 5m 51s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 4m 24s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 2m 37s | View ↗ |
nx run-many -t lint -p @tryghost/shade,@tryghos... |
✅ Succeeded | 2m 6s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 1m 4s | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 6s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-06 17:52:56 UTC
WalkthroughChangesThe change modifies how the Related PRs: None identified. Suggested labels: bug, dependencies Suggested reviewers: None identified. PoemA rabbit hopped through bundler maze, 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (3)
apps/shade/src/components/ui/flag.tsx (3)
3-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify test coverage exercises both export shapes.
This fix's whole purpose is handling two divergent module shapes (fully-unwrapped default vs. Rolldown's wrapper-preserving default). The referenced unit test suite runs under a single bundler, so it likely only exercises one branch of
interopDefault. Consider mockingreact-world-flagswith both shapes (plain component, and{__esModule: true, default: {__esModule: true, default: Component}}) to lock in the fix and guard against regressions.🤖 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 `@apps/shade/src/components/ui/flag.tsx` around lines 3 - 18, The interop logic in interopDefault and its ReactFlag usage should be covered by tests for both module shapes, since the current suite likely only exercises one bundler behavior. Update the unit tests for flag.tsx to mock react-world-flags as a plain component export and as the nested wrapper shape ({ __esModule: true, default: { __esModule: true, default: Component } }) and assert ReactFlag resolves correctly in both cases.
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve
FlagPropstyping instead ofRecord<string, unknown>.
react-world-flags's types (@types/react-world-flags@1.6.0) declare a realFlagProps(code,fallback, plus passthroughHTMLProps<HTMLImageElement>). TypingReactFlagasReact.ComponentType<Record<string, unknown>>drops that contract, so a typo or wrong value type on the JSX usage below (Lines 42-48) would silently compile.♻️ Proposed fix to retain prop typing
+type FlagComponentProps = React.ComponentProps<typeof ReactWorldFlags.default>; -const ReactFlag = interopDefault<React.ComponentType<Record<string, unknown>>>(ReactWorldFlags); +const ReactFlag = interopDefault<React.ComponentType<FlagComponentProps>>(ReactWorldFlags);🤖 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 `@apps/shade/src/components/ui/flag.tsx` at line 18, The ReactWorldFlags wrapper is being typed too loosely in ReactFlag, which drops the real FlagProps contract and allows invalid JSX props to compile silently. Update the interopDefault typing in the flag component to preserve the library’s FlagProps (including code, fallback, and HTMLProps<HTMLImageElement> passthrough) so the Flag usage in the same component continues to get proper type checking.
9-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo fallback guard if unwrap fails to find a usable component.
If a future bundler/runtime returns a shape neither branch anticipates (e.g., a third wrapping layer),
interopDefaultsilently returns that value viaas T, reproducing the exact crash this PR fixes, just later and harder to trace. Consider a lightweight dev-only assertion so a bad shape fails loudly at the source instead of surfacing as a cryptic React render error.🛡️ Proposed dev-time guard
if (value && typeof value === 'object' && '__esModule' in value && 'default' in value) { value = (value as {default: unknown}).default; } + if (process.env.NODE_ENV !== 'production' && !value) { + // eslint-disable-next-line no-console + console.error('interopDefault: unable to resolve a component from module', mod); + } return value as T;🤖 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 `@apps/shade/src/components/ui/flag.tsx` around lines 9 - 17, The interopDefault helper in flag.tsx can still return an unusable value when module shapes are unexpectedly nested, so add a lightweight dev-only guard in interopDefault that validates the final unwrapped value before casting. If the resolved value is not a valid component-like export, fail loudly with a clear assertion/error instead of returning it through as T, so bad bundler/runtime shapes are caught at the source.
🤖 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 `@apps/shade/src/components/ui/flag.tsx`:
- Around line 3-18: The interop logic in interopDefault and its ReactFlag usage
should be covered by tests for both module shapes, since the current suite
likely only exercises one bundler behavior. Update the unit tests for flag.tsx
to mock react-world-flags as a plain component export and as the nested wrapper
shape ({ __esModule: true, default: { __esModule: true, default: Component } })
and assert ReactFlag resolves correctly in both cases.
- Line 18: The ReactWorldFlags wrapper is being typed too loosely in ReactFlag,
which drops the real FlagProps contract and allows invalid JSX props to compile
silently. Update the interopDefault typing in the flag component to preserve the
library’s FlagProps (including code, fallback, and HTMLProps<HTMLImageElement>
passthrough) so the Flag usage in the same component continues to get proper
type checking.
- Around line 9-17: The interopDefault helper in flag.tsx can still return an
unusable value when module shapes are unexpectedly nested, so add a lightweight
dev-only guard in interopDefault that validates the final unwrapped value before
casting. If the resolved value is not a valid component-like export, fail loudly
with a clear assertion/error instead of returning it through as T, so bad
bundler/runtime shapes are caught at the source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ef0a0f07-62dc-40e2-88c9-05fbffd5b17a
📒 Files selected for processing (1)
apps/shade/src/components/ui/flag.tsx
ref https://linear.app/ghost/issue/PLA-96/spike-rolldown-vite-drop-in-build-benchmarks country-flag-icons is true ESM, so the Flag component no longer needs the CJS interop shim added in #29137 to work around react-world-flags's {__esModule, default} shape under Rolldown; supersedes #29137.
1 participant
ref https://linear.app/ghost/issue/PLA-96/spike-rolldown-vite-drop-in-build-benchmarks
What
Shade's
Flagcomponent importsreact-world-flags, a CommonJS module shaped{ __esModule: true, default: <Component> }. It replaces the bare default import with a namespace import plus an explicitinteropDefault()unwrap.Why
esbuild and Rollup unwrap the CJS default down to the component automatically, so
<ReactFlag/>works today. The Rolldown bundler (being evaluated for the admin apps under PLA-96) instead hands back the__esModulewrapper object — whose.defaultis still the wrapper — so<ReactFlag/>renders an object and React throwsElement type is invalid: got object, crashing the analytics location/source views that useFlag.The fix
interopDefault()reads.defaultoff the module (namespace or CJS export), and if that value is itself still an__esModule-tagged wrapper, unwraps one more level. This resolves to the actual component under both toolchains:.defaultyields the component, second branch is a no-op.Scope
apps/shade/src/components/ui/flag.tsx. The<ReactFlag .../>JSX usage is unchanged.Verification (Rollup toolchain)
pnpm --filter @tryghost/shade build— clean (includestsc+tsc-alias+ vite build)pnpm --filter @tryghost/shade lint— cleanpnpm --filter @tryghost/shade test:unit— 226 passed (includestsc --noEmittypecheck)