feat(init): close the framework scaffolding gaps for expo, express, fastify, ios, and android#394
Open
rafa-thayto wants to merge 6 commits into
Open
feat(init): close the framework scaffolding gaps for expo, express, fastify, ios, and android#394rafa-thayto wants to merge 6 commits into
rafa-thayto wants to merge 6 commits into
Conversation
…astify, ios, and android - Expo: wrap the expo-router root layout with ClerkProvider + token cache, and add an Expo starter to the bootstrap registry (create-expo-app) - Express/Fastify: wire clerkMiddleware()/clerkPlugin into the server entry (ESM and CJS) via a shared node-server scaffolder, plus the @clerk/express request type augmentation for TypeScript projects - iOS/Android: detect native platforms via marker files (*.xcodeproj, AndroidManifest.xml), accept --framework ios|android, skip the npm SDK install, and print the exact SDK quickstart steps
Fixes found by adversarial review and hands-on verification of the framework scaffolding, each with a regression test written first: - mask comments/strings so commented-out or quoted app-creation statements can't hijack the Express/Fastify middleware insertion - scope the Expo ClerkProvider wrap to the default export's body (sibling exports like ErrorBoundary are no longer wrapped) and wrap the last single-line return instead of the first (guards untouched) - insertAfterLastImport spans full multi-line import statements instead of splicing into them - skip wiring only when the framework's own SDK is present — an unrelated @clerk/* import no longer false-skips - wildcard native markers (*.xcodeproj/*.xcworkspace) must be directories; a stray plain file no longer misdetects iOS - creation regexes accept type-annotated declarations (const app: Express = express()) - emit the quickstart post-instruction when an entry file exists but no creation call is found, matching the README - align added imports with the codebase style (brace spacing, order) - iOS instructions include the official .environment(Clerk.shared) SwiftUI injection step
🦋 Changeset detectedLatest commit: 827f498 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 |
|
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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 3
🤖 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/cli-core/src/commands/init/frameworks/node-server.ts`:
- Around line 100-102: Update isCommonJs and the “already configured” check in
the initialization flow to inspect the existing masked content rather than raw
content, preventing comments and strings from triggering detection. Compute or
retain masked once before these checks, pass it into isCommonJs, and reuse the
same masked value for the creation-pattern match.
In `@packages/cli-core/src/commands/init/frameworks/transformations.ts`:
- Around line 41-78: Update maskCommentsAndStrings to parse template literals
with nested ${...} substitutions instead of treating every backtick as a simple
string delimiter. Track template-expression brace depth and recursively handle
strings, comments, and nested templates within substitutions, while preserving
masking and source-length alignment so downstream callers such as
insertAfterLastImport, scaffoldServerEntry, wrapLastReturnWithProvider, and
findDefaultExportBody continue matching code after nested templates.
In `@packages/cli-core/src/commands/init/README.md`:
- Around line 90-97: Update the native mobile platform detection introduction in
the README to avoid asserting that native repositories lack package.json files.
State that they may not have one, or focus on marker-based fallback detection
when no npm framework matches, while preserving the existing marker and SDK
installation guidance.
🪄 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 Plus
Run ID: 37840c8f-a5a3-4e0c-960b-42e9b98493f0
📒 Files selected for processing (23)
.changeset/init-framework-scaffolding-gaps.mdpackages/cli-core/src/commands/init/README.mdpackages/cli-core/src/commands/init/bootstrap-registry.tspackages/cli-core/src/commands/init/bootstrap.test.tspackages/cli-core/src/commands/init/frameworks/android.test.tspackages/cli-core/src/commands/init/frameworks/android.tspackages/cli-core/src/commands/init/frameworks/expo.test.tspackages/cli-core/src/commands/init/frameworks/expo.tspackages/cli-core/src/commands/init/frameworks/express.test.tspackages/cli-core/src/commands/init/frameworks/express.tspackages/cli-core/src/commands/init/frameworks/fastify.test.tspackages/cli-core/src/commands/init/frameworks/fastify.tspackages/cli-core/src/commands/init/frameworks/ios.test.tspackages/cli-core/src/commands/init/frameworks/ios.tspackages/cli-core/src/commands/init/frameworks/node-server.test.tspackages/cli-core/src/commands/init/frameworks/node-server.tspackages/cli-core/src/commands/init/frameworks/transformations.test.tspackages/cli-core/src/commands/init/frameworks/transformations.tspackages/cli-core/src/commands/init/index.test.tspackages/cli-core/src/commands/init/index.tspackages/cli-core/src/commands/init/scaffold.tspackages/cli-core/src/lib/framework.test.tspackages/cli-core/src/lib/framework.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
…for CJS detection
- maskCommentsAndStrings now recursively scans `${...}` substitutions
as real code instead of a naive backtick-to-backtick match, so a
nested template literal can no longer desynchronize delimiter
matching and corrupt masking for the rest of the file.
- scaffoldServerEntry's isCommonJs check now runs against masked
content so a commented-out `require(...)` can't mis-classify a
server entry file as CommonJS.
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/cli-core/src/commands/init/frameworks/transformations.test.ts`:
- Around line 51-84: Strengthen the maskCommentsAndStrings parameterized
fixtures by adding code-like text after each embedded delimiter: a fake
require(...) after the nested template and identifiers following the
string/comment braces inside ${...}. Update the corresponding mustNotContain or
mustContain assertions so text still inside the template is masked while
REAL_CODE outside it remains preserved, protecting CommonJS detection.
🪄 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 Plus
Run ID: 197ec69c-fe1f-43bc-b523-119d06f2b5a2
📒 Files selected for processing (4)
packages/cli-core/src/commands/init/README.mdpackages/cli-core/src/commands/init/frameworks/node-server.tspackages/cli-core/src/commands/init/frameworks/transformations.test.tspackages/cli-core/src/commands/init/frameworks/transformations.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cli-core/src/commands/init/frameworks/node-server.ts
- packages/cli-core/src/commands/init/README.md
- packages/cli-core/src/commands/init/frameworks/transformations.ts
Comment on lines
+51
to
+84
| describe("maskCommentsAndStrings", () => { | ||
| test.each([ | ||
| { | ||
| name: "blanks the literal text inside a nested template substitution", | ||
| source: "const a = `outer ${`inner`} end`;\nconst REAL_CODE = 1;\n", | ||
| mustNotContain: "inner", | ||
| mustContain: "REAL_CODE", | ||
| }, | ||
| { | ||
| name: "keeps an object literal's braces from ending the substitution early", | ||
| source: "const a = `x ${({ b: 1 }).b} y`;\nconst REAL_CODE = 1;\n", | ||
| mustNotContain: "x ${", | ||
| mustContain: "({ b: 1 }).b", | ||
| }, | ||
| { | ||
| name: "handles a string literal inside a substitution", | ||
| source: 'const a = `x ${"}"} y`;\nconst REAL_CODE = 1;\n', | ||
| mustNotContain: '"}"', | ||
| mustContain: "REAL_CODE", | ||
| }, | ||
| { | ||
| name: "handles a comment inside a substitution", | ||
| source: "const a = `x ${/* } */ 1} y`;\nconst REAL_CODE = 1;\n", | ||
| mustNotContain: "} */", | ||
| mustContain: "REAL_CODE", | ||
| }, | ||
| ])("$name", ({ source, mustNotContain, mustContain }) => { | ||
| const masked = maskCommentsAndStrings(source); | ||
|
|
||
| expect(masked).not.toContain(mustNotContain); | ||
| expect(masked).toContain(mustContain); | ||
| expect(masked.length).toBe(source.length); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that code-like text after embedded delimiters remains masked.
These fixtures mostly check that the delimiter-containing literal/comment is absent and that REAL_CODE after the template survives. A broken scanner could still terminate early and leak text from the remainder of the template while passing these assertions. Add a fake require(...) after the nested template, plus identifiers after the embedded string/comment braces inside ${...}, and assert those are masked/preserved as appropriate. This directly protects the CommonJS-detection hardening described in the 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 `@packages/cli-core/src/commands/init/frameworks/transformations.test.ts`
around lines 51 - 84, Strengthen the maskCommentsAndStrings parameterized
fixtures by adding code-like text after each embedded delimiter: a fake
require(...) after the nested template and identifiers following the
string/comment braces inside ${...}. Update the corresponding mustNotContain or
mustContain assertions so text still inside the template is masked while
REAL_CODE outside it remains preserved, protecting CommonJS detection.
An apostrophe in ordinary JSX text (`Don't`) or a quote inside a regex literal (`/"/g`) opened a "string" that never closed, hiding the rest of the file from the scaffolder. Real JS string literals never span lines, so a quote with no partner on the same line is punctuation, not a delimiter; template literals keep their multi-line handling. Symptoms this fixes, each covered by a regression test: - an Expo layout containing an apostrophe silently got no ClerkProvider - with semicolon-less code plus an apostrophe in a trailing comment, the Express/Fastify middleware was inserted after `app.listen()`, so auth never applied to any route Both hand-written scanners (findMatchingDelimiter, findStatementEnd) now reuse the hardened mask instead of duplicating their own quote tracking.
The stock React Router root.tsx ends with a multi-line
`{ Links, Meta, Outlet, Scripts, ScrollRestoration }` import. The old
insertAfterLastImport spliced `export const middleware` into the middle
of that brace block, producing invalid syntax — and the corruption
cascaded, since every later magicast parse then failed and silently fell
back to prepending more imports.
The hardened helper already fixes this; this test fails against the old
implementation and pins the behavior so it can't regress.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli-core/src/commands/init/frameworks/transformations.test.ts (2)
106-109: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winClose the
${...}substitution in this fixture.The
}in"}"is inside the quoted string, so this source has no substitution-closing brace. The masker therefore consumes the outer backtick and masksREAL_CODE, causing this test to fail instead of testing a string inside a valid substitution.Proposed fix
- source: 'const a = `x ${"}" y`;\nconst REAL_CODE = 1;\n', + source: 'const a = `x ${"}"} y`;\nconst REAL_CODE = 1;\n',🤖 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/cli-core/src/commands/init/frameworks/transformations.test.ts` around lines 106 - 109, Update the “handles a string literal inside a substitution” fixture so the template literal contains a valid closing brace after the quoted string, while keeping the quoted “}” inside the substitution. Preserve the existing REAL_CODE and mustNotContain assertions so the test specifically exercises masking a string literal within a closed substitution.
79-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that insertion follows the real import.
This assertion also passes if
SNIPPETis prepended to the file—or omitted (indexOfreturns-1). Assert the exact expected output or compare against the end of the actual import line.Proposed fix
- expect(result.indexOf("SNIPPET")).toBeLessThan(result.indexOf("// import")); + expect(result).toBe(`import { a } from "a"; +SNIPPET +// import { b } from "b"; (removed) +const x = 1; +`);🤖 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/cli-core/src/commands/init/frameworks/transformations.test.ts` around lines 79 - 88, Strengthen the test for insertAfterLastImport by asserting the exact expected output, or verifying that SNIPPET starts immediately after the real `import { a } from "a";` line and before the comment. Avoid relying only on index ordering, which also permits prepending or omitting the snippet.
🤖 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.
Outside diff comments:
In `@packages/cli-core/src/commands/init/frameworks/transformations.test.ts`:
- Around line 106-109: Update the “handles a string literal inside a
substitution” fixture so the template literal contains a valid closing brace
after the quoted string, while keeping the quoted “}” inside the substitution.
Preserve the existing REAL_CODE and mustNotContain assertions so the test
specifically exercises masking a string literal within a closed substitution.
- Around line 79-88: Strengthen the test for insertAfterLastImport by asserting
the exact expected output, or verifying that SNIPPET starts immediately after
the real `import { a } from "a";` line and before the comment. Avoid relying
only on index ordering, which also permits prepending or omitting the snippet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c8101054-00a5-4beb-831d-df9fa450f6d5
📒 Files selected for processing (7)
packages/cli-core/src/commands/init/frameworks/expo.test.tspackages/cli-core/src/commands/init/frameworks/expo.tspackages/cli-core/src/commands/init/frameworks/express.test.tspackages/cli-core/src/commands/init/frameworks/node-server.test.tspackages/cli-core/src/commands/init/frameworks/node-server.tspackages/cli-core/src/commands/init/frameworks/transformations.test.tspackages/cli-core/src/commands/init/frameworks/transformations.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/cli-core/src/commands/init/frameworks/node-server.test.ts
- packages/cli-core/src/commands/init/frameworks/node-server.ts
- packages/cli-core/src/commands/init/frameworks/expo.ts
- packages/cli-core/src/commands/init/frameworks/express.test.ts
- packages/cli-core/src/commands/init/frameworks/transformations.ts
- packages/cli-core/src/commands/init/frameworks/expo.test.ts
…r scaffolders
Quality pass over the scaffolding changes on this branch.
- extract maskCommentsAndStrings + findMatchingDelimiter into source-scan.ts.
The masker is the subtlest code here — it carries an index-alignment
invariant three modules depend on — and it was sitting in a grab-bag file
next to Tailwind header markup. expo.ts also kept a private copy of the
delimiter scanner; there is now one implementation.
- replace needsManualWiring, which drove control flow by matching the prose
of a skip reason ("Could not find…"), with an explicit `wired` flag
returned by scaffoldServerEntry. Rewording a user-facing message can no
longer silently drop a setup instruction.
- move plan assembly into scaffoldServerFramework so express.ts and
fastify.ts stop duplicating it. The quickstart URL and manual-wiring
sentence are now config, so fastify.ts is pure configuration and
express.ts only adds its types/globals.d.ts action.
- findStatementEnd takes masked source like the other scanners instead of
re-masking content its caller had already masked.
No behavior change: 2217 tests pass, and the real create-expo-app project
still scaffolds to identical output.
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
1 participant
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
<ClerkProvider>+ secure token cache (existing layouts are modified in place, missing ones created), and add an Expo starter to the bootstrap registry soclerk init --starter --framework exposcaffolds a new app viacreate-expo-appclerkMiddleware()/clerkPlugininto the server entry file (ESM and CJS, including inline-require and type-annotated forms) via a sharednode-server.tsscaffolder; TypeScript Express apps also get the@clerk/express/envrequest type augmentation*.xcodeproj/*.xcworkspacedirectory bundles,AndroidManifest.xml), accept--framework ios|android, skip the npm bootstrap/install paths, and print the exact SDK quickstart steps (validated against the official Clerk docs, including the SwiftUI.environment(Clerk.shared)step)ErrorBoundaryexports are no longer wrapped), multi-line-import-safe insertion, directory-only wildcard markers, and style-consistent generated imports — every fix has a regression test written firstTest Plan
bun run format:check/bun run lint/bun run typecheck/bun run test— 2201 tests pass (28 new regression tests on this branch)x-clerk-auth-statusheaders stamped by the live middlewarecreate-expo-apprun with the registry's exact flags, chained into the scaffolder;tsc --noEmitdelta vs pristine template = zero;expo export --platform webbundles and renders with clerk-js booting in-browserPackage.swift,build.gradle.kts, stray.xcodeprojfile) correctly not detectedNotes
No releasing changes; changeset included (
minor). Native platforms are instruction-only by design — their SDKs (SPM/Gradle) can't be safely installed by a JS package manager, so init links the app, pulls keys, and prints the documented setup steps instead.