Skip to content

Conversation

@nlynzaad
Copy link
Contributor

@nlynzaad nlynzaad commented Nov 25, 2025

This PR resolves #5658.

When using the plugin, the inline config is passed to the generator and processed together with the tsr.config.json config (if it exists). This includes resolving any relative paths.

The inline config, however, may contain fields other than what is needed and returned from the generator config and hence needs to be parsed in addition to the generator config.

Currently, the inline config is merged into the resolved generator config overwriting any resolved fields from there.

This PR changes this behavior by reversing this and merging the generator config into the inline config thereby overwriting the inline config with the processed fields from the generator where relevant.

Additionally, this PR adds a few unit tests to ensure the configuration is read and processed correctly going forward.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected configuration property precedence during merge operations to ensure file-based configuration settings properly override inline configurations.
  • Tests

    • Added comprehensive test coverage for configuration merging behavior across various path formats and configuration source combinations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Walkthrough

This pull request fixes a configuration merge precedence issue in the router plugin. The merge order of inline and external configs is swapped in getConfig(), ensuring that external config properties (e.g., from Vite root) take precedence over inline config, alongside corresponding test coverage and configuration fixtures.

Changes

Cohort / File(s) Summary
Config merge precedence fix
packages/router-plugin/src/core/config.ts
Reversed object spread order from { ...config, ...inlineConfig } to { ...inlineConfig, ...config }, changing precedence so external config overrides inline config during merge
Test coverage for config merging
packages/router-plugin/tests/config.test.ts
New comprehensive test suite covering getConfig behavior with multiple scenarios combining inline and JSON configs, testing various path forms and root path resolution
Test configuration fixtures
packages/router-plugin/tests/config/withJson/tsr.config.json
New test configuration file specifying routes directory, generated route tree output, target framework, and code splitting settings

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The config.ts change is a straightforward merge order reversal with clear intent
  • The test file introduces new coverage but follows a consistent test pattern across multiple similar cases
  • Test fixture is a simple JSON configuration file

Suggested reviewers

  • schiller-manuel
  • Sheraff

Poem

🐰 A rabbit's ode to proper precedence:

When configs collide in the merge,
The outer must take charge and surge,
No more shall inline override the way,
The Vite root paths now hold the day! 🎯

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: reversing the merge order to prevent inline config from overwriting resolved generator config fields.
Linked Issues check ✅ Passed The pull request addresses the core issue by reversing merge order so resolved generator config fields are not overwritten by inline config, which aligns with the linked issue requirement to fix path resolution relative to Vite root.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing configuration merge order: config.ts implements the fix, config.test.ts provides test coverage, and tsr.config.json serves as test fixture data.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 5658

📜 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 d2f7ff4 and 1319827.

📒 Files selected for processing (3)
  • packages/router-plugin/src/core/config.ts (1 hunks)
  • packages/router-plugin/tests/config.test.ts (1 hunks)
  • packages/router-plugin/tests/config/withJson/tsr.config.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety throughout the codebase

Files:

  • packages/router-plugin/src/core/config.ts
  • packages/router-plugin/tests/config.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.258Z
Learning: Applies to packages/solid-router/**/*.{ts,tsx} : Solid Router components and primitives should use the tanstack/solid-router package
📚 Learning: 2025-11-25T00:18:21.258Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.258Z
Learning: Applies to packages/solid-router/**/*.{ts,tsx} : Solid Router components and primitives should use the tanstack/solid-router package

Applied to files:

  • packages/router-plugin/src/core/config.ts
  • packages/router-plugin/tests/config/withJson/tsr.config.json
📚 Learning: 2025-11-25T00:18:21.258Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.258Z
Learning: Applies to **/src/routes/**/*.{ts,tsx} : Use file-based routing in src/routes/ directories or code-based routing with route definitions

Applied to files:

  • packages/router-plugin/src/core/config.ts
  • packages/router-plugin/tests/config.test.ts
  • packages/router-plugin/tests/config/withJson/tsr.config.json
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • packages/router-plugin/tests/config.test.ts
  • packages/router-plugin/tests/config/withJson/tsr.config.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.

Applied to files:

  • packages/router-plugin/tests/config/withJson/tsr.config.json
🧬 Code graph analysis (1)
packages/router-plugin/tests/config.test.ts (2)
packages/router-plugin/src/core/config.ts (2)
  • Config (111-111)
  • getConfig (105-109)
packages/router-core/src/route.ts (1)
  • path (1553-1555)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test
🔇 Additional comments (3)
packages/router-plugin/src/core/config.ts (1)

105-109: LGTM! Merge order fix correctly resolves the path resolution issue.

The swapped merge order ensures that processed fields from the generator (including resolved paths) take precedence over raw inline config values, while still preserving any extra fields in inlineConfig that the generator doesn't process. This fixes the reported issue where resolved paths were being overwritten by unresolved inline paths.

packages/router-plugin/tests/config/withJson/tsr.config.json (1)

1-6: LGTM! Valid test fixture for verifying JSON config merge behavior.

The configuration values are appropriate for testing the merge logic when both JSON and inline configs are present.

packages/router-plugin/tests/config.test.ts (1)

1-112: LGTM! Comprehensive test coverage for config merge behavior.

The test suite thoroughly validates getConfig with various combinations of inline and JSON configurations, including different path formats (relative, "./", absolute). The assertion logic correctly expects inline config to take precedence over JSON config for overlapping fields, with resolved absolute paths overriding both.


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

@nx-cloud
Copy link

nx-cloud bot commented Nov 25, 2025

View your CI Pipeline Execution ↗ for commit 1319827

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 6m 42s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-25 17:49:48 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 25, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5963

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5963

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5963

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5963

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5963

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5963

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5963

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5963

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5963

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5963

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5963

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5963

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5963

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5963

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5963

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5963

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5963

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5963

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5963

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5963

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5963

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5963

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5963

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@5963

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5963

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5963

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5963

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5963

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5963

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5963

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5963

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5963

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5963

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5963

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5963

commit: 1319827

@nlynzaad nlynzaad merged commit 2ae3702 into main Nov 25, 2025
6 checks passed
@nlynzaad nlynzaad deleted the 5658 branch November 25, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

routeTree generation breaks when using different root in vite config

2 participants