Skip to content

Conversation

@beaussan
Copy link
Contributor

@beaussan beaussan commented Oct 21, 2025

Given vite only dedupe when a dependancy is an explicit dependancy, and not a peer dependancy, this lead to multiple solidjs version if you used other tanstack devtools

Summary by CodeRabbit

  • Chores
    • Refined dependency management to remove duplicate entries and streamline package installation.
    • Adjusted build/bundling configuration so certain libraries are no longer explicitly bundled, reducing package size and simplifying distribution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

Moved solid-js into dependencies and removed it from devDependencies and peerDependencies in router-devtools-core; also removed solid-js and solid-js/web from the Vite bundledDeps configuration.

Changes

Cohort / File(s) Summary
Dependency edits
packages/router-devtools-core/package.json
Added solid-js under dependencies (^1.9.5); removed solid-js from devDependencies and peerDependencies.
Build config update
packages/router-devtools-core/vite.config.ts
Removed ['solid-js', 'solid-js/web'] from tanstackViteConfig bundledDeps, altering which packages are explicitly bundled by the build.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Developer
  participant Vite as Vite Build
  participant Rollup as Rollup/Bundler
  participant NodeModules as node_modules

  Note over Developer,Vite: Before
  Developer->>Vite: build router-devtools-core
  Vite->>Rollup: include bundledDeps ['solid-js','solid-js/web']
  Rollup->>NodeModules: bundle `solid-js` and `solid-js/web`
  Rollup-->>Vite: bundled artifacts

  Note over Developer,Vite: After (this PR)
  Developer->>Vite: build router-devtools-core
  Vite->>Rollup: bundledDeps does NOT include solid-js
  Rollup->>NodeModules: treat `solid-js` as regular dependency (external/normal resolution)
  Rollup-->>Vite: artifacts without explicit bundling of solid-js
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • birkskyum
  • lachlancollins

Poem

🐰 In the repo meadow I hop with glee,
I nudged solid-js where it needs to be,
From peer and dev it hopped to depend,
Build winds changed how it's bundled, my friend —
A tiny tweak, a stable spree! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: move solid-js to prod dep on devtools-core" directly corresponds to the primary changes in the changeset. The raw summary confirms that solid-js was moved from devDependencies and peerDependencies to dependencies in the package.json, which aligns precisely with the title's description. The title uses the "fix:" prefix appropriately since the PR addresses the issue where Vite can only dedupe explicit dependencies, not peer dependencies. The title is concise, clear, and free of vague language or noise, making it easy for a teammate to understand the core change from git history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 a204c68 and 559a559.

📒 Files selected for processing (2)
  • packages/router-devtools-core/package.json (1 hunks)
  • packages/router-devtools-core/vite.config.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • packages/router-devtools-core/vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/router-devtools-core/package.json
⏰ 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). (2)
  • GitHub Check: Test
  • GitHub Check: Preview

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.

@nx-cloud
Copy link

nx-cloud bot commented Oct 21, 2025

View your CI Pipeline Execution ↗ for commit 559a559

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

☁️ Nx Cloud last updated this comment at 2025-10-22 08:23:14 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 21, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: 559a559

@beaussan beaussan changed the title chore: move solid-js to prod dep on devtools-core fix: move solid-js to prod dep on devtools-core Oct 22, 2025
@beaussan beaussan marked this pull request as ready for review October 22, 2025 08:11
@AlemTuzlak
Copy link
Contributor

This is a fix for:
TanStack/devtools#183

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eab8a99 and a204c68.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • packages/router-devtools-core/package.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Use workspace:* protocol for internal dependencies in package.json files

Files:

  • packages/router-devtools-core/package.json
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

@birkskyum
Copy link
Member

Does this effectively revert the changes here?

@benmccann , what do you think of this change?

@benmccann
Copy link
Contributor

Yes, this reverts the bundling. I do agree that not bundling is better for solid users. However, that's less than 1% of users. And not bundling is worse for everyone else. So I think on balance that we're better off with the bundling. I included some stats about that in #5374

Is the concern with multiple copies of solid related to size or something else?

@beaussan
Copy link
Contributor Author

I don't have the full context on the changes, @AlemTuzlak doses have way more info around it.

From TanStack/devtools#183, it seems like the issue around multiple solid js version is not related to bundle size, but to have multiple version of solid-js within warnings in the console and could (from my limited solid-js architecture knowledge) cause issues.

image

I have doubts however if this is actually causing issues given we mount each devtools into it's own solid-js render tree

By bundling solid we end up with a much smaller package since we keep only the parts of it that are used (~40kb) and tree-shake away the rest. This prevents deduping for solid users, so it's larger for those users. However, there are 3,700 weekly downloads from solid and 1 million from react, so this is a huge savings overall.

Are we talking only about npm install size? It shouldn't be in production (unless you use the devtools in production)

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.

4 participants