Skip to content

fix: detect Next.js route groups as valid project structure#3090

Open
mvanhorn wants to merge 1 commit intoonlook-dev:mainfrom
mvanhorn:osc/2936-fix-nextjs-route-groups-validation
Open

fix: detect Next.js route groups as valid project structure#3090
mvanhorn wants to merge 1 commit intoonlook-dev:mainfrom
mvanhorn:osc/2936-fix-nextjs-route-groups-validation

Conversation

@mvanhorn
Copy link
Copy Markdown

@mvanhorn mvanhorn commented Mar 15, 2026

Summary

Next.js projects using route groups (parenthesized directories like (app), (dashboard)) were rejected during project import because the validator only checked for layout files at the exact app/ or src/app/ path level.

Changes

Modified isTargetFile in packages/utility/src/path.ts to also match files inside Next.js route group subdirectories. A route group directory matches the (name) pattern per the Next.js App Router spec.

For example, app/(marketing)/layout.tsx now correctly matches the potential path app.

Added 5 test cases covering route group paths, including edge cases for nested directories and non-group directories.

Testing

  • All 36 existing + new tests pass (bun test packages/utility/test/path.test.ts)
  • Typecheck passes (bun run typecheck)
  • Verified that non-route-group subdirectories (e.g., app/marketing/layout.tsx) are still correctly rejected

Fixes #2936

This contribution was developed with AI assistance (Claude Code).

Summary by CodeRabbit

  • New Features

    • Improved recognition of Next.js route group subdirectories in file path handling.
  • Tests

    • Added test coverage for layout files within route groups, including edge cases and nested scenarios.

The project validator failed to recognize Next.js projects using route
groups (parenthesized directories like (app), (dashboard)). The
isTargetFile function did an exact directory match against potential
paths, missing layout files inside route group subdirectories.

Added route group pattern matching to isTargetFile so that paths like
app/(marketing)/layout.tsx match the potentialPath 'app'.

Fixes onlook-dev#2936
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 15, 2026

@mvanhorn is attempting to deploy a commit to the Onlook Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eb9695d9-d62e-402c-ae48-97e4f786d2c4

📥 Commits

Reviewing files that changed from the base of the PR and between a242be5 and e9c1d3e.

📒 Files selected for processing (2)
  • packages/utility/src/path.ts
  • packages/utility/test/path.test.ts

📝 Walkthrough

Walkthrough

Enhances the isTargetFile utility function to recognize Next.js route group subdirectories by matching parent directories against potential paths and validating route group patterns like "(...)". Includes comprehensive test coverage for layout files within route groups and edge cases.

Changes

Cohort / File(s) Summary
Path Utility Enhancement
packages/utility/src/path.ts
Extends isTargetFile logic to detect Next.js route group patterns by checking if the parent directory matches the potential path and the last segment follows the "(groupName)" pattern.
Path Utility Tests
packages/utility/test/path.test.ts
Adds test cases for isRootLayoutFile validating route group recognition in app/(marketing)/, src/app/(dashboard)/, handling .jsx extensions, and rejecting deeply nested or non-standard patterns.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A hop, skip, and a route group check,
Now nested layouts work, what the heck!
Parentheses patterns we now understand,
Next.js projects flourish across the land!
Route groups matter—let's give a cheer,
Valid projects arrive loud and clear! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: enhancing Next.js route group detection in the project validator.
Description check ✅ Passed The description covers the problem, solution, testing approach, and issue reference. It follows the template structure with clear sections.
Linked Issues check ✅ Passed The PR successfully addresses issue #2936 by modifying isTargetFile to recognize route group subdirectories and adding comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the route group validation issue. The modifications to path.ts and path.test.ts are focused and within scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Nextjs route groups are not considered as a valid project

1 participant