Uh oh!
There was an error while loading. Please reload this page.
fix(expo): Preserve platform-specific imports in tsdown builds - #8865
Conversation
🦋 Changeset detectedLatest commit: 8a58e59 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughA new Rolldown plugin ChangesExpo Platform Specifier Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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 docstrings
Comment |
| outDir: './dist', | ||
| entry: ['./src/**/*.{ts,tsx,js,jsx}', '!./src/**/*.test.{ts,tsx}', '!./src/**/__tests__/**'], | ||
| bundle: false, | ||
| unbundle: true, |
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
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 `@packages/expo/scripts/preservePlatformSpecifiers.mts`:
- Around line 45-55: The guard condition at line 45 does not reject sources with
explicit file extensions (like .js or .ts), allowing extensioned imports such as
./foo.js to pass through and be externalized at line 55 when platform siblings
exist. This violates the plugin's constraint to only handle extensionless
imports. Add a check to the condition at line 45 that rejects any source with an
explicit file extension (by detecting a dot followed by an extension pattern) so
only truly extensionless imports like ./foo can be processed.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f29cc734-d2aa-4659-b83a-4266bcf20a28
📒 Files selected for processing (4)
.changeset/expo-platform-imports.mdpackages/expo/scripts/__tests__/preservePlatformSpecifiers.test.tspackages/expo/scripts/preservePlatformSpecifiers.mtspackages/expo/tsdown.config.mts
Uh oh!
There was an error while loading. Please reload this page.
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Uh oh!
There was an error while loading. Please reload this page.
Description
Fixes Expo platform-specific module resolution regression introduced by the tsup to tsdown migration (ref).
With tsup, bundleless CJS output preserved extensionless internal imports (ref):
After the tsdown migration, Rolldown's preserveModules output rewrites those imports to emitted filenames (ref):
That bypasses Metro's platform-specific resolution (ref), which needs the extensionless specifier so it can select files like
useSignInWithApple.ios.js,NativeClerkModule.web.js, etc.This PR keeps the tsdown build and adds an Expo-local Rolldown plugin that preserves only the relative imports whose resolved source has platform-specific siblings. Normal internal imports still point at emitted
.jschunks.The alternative would be reverting
@clerk/expoto tsupChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit