Conversation
📦 Bundle Size Comparison📈 nuxi
📈 nuxt-cli
➡️ create-nuxt
|
📝 WalkthroughWalkthroughThis pull request introduces an executor abstraction for the typecheck command, enabling selection between vue-tsc and golar type-checking backends. The implementation adds a command argument for executor selection, type guards to validate executor values, and a resolveExecutor function mapping executors to their CLI entry points. Hard-coded vue-tsc references are replaced with dynamic executor resolution. Bun and npx invocation logic is updated to install and run the selected executor and its dependencies. An experimental warning is logged when using the golar backend. A test:tscheck script is added to playground/package.json. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. 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: 1
🧹 Nitpick comments (2)
packages/nuxi/src/commands/typecheck.ts (2)
98-100:isExecutorduplicates the executor list — derive it fromexecutorPackagesinstead.The type guard hardcodes
'vue-tsc' | 'golar', duplicating the keys already inexecutorPackages. If a new executor is added to the map, this guard must be updated in sync.Proposed refactor
function isExecutor(value: unknown): value is Executor { - return value === 'vue-tsc' || value === 'golar' + return typeof value === 'string' && value in executorPackages }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/src/commands/typecheck.ts` around lines 98 - 100, The isExecutor type guard currently hardcodes 'vue-tsc' and 'golar'; change it to derive allowed executors from the keys of executorPackages so it stays in sync: implement isExecutor(value: unknown): value is Executor by checking typeof value === 'string' and that (Object.keys(executorPackages) as string[]).includes(value), using executorPackages (the existing map) and returning that boolean so the guard reflects the map dynamically.
67-79: Bun global install is a significant side effect.
bun install --globalmodifies the user's global environment. If the user has a different version of these packages installed globally, this could cause conflicts. This may have been the pre-existing behavior forvue-tsc, but it's worth noting that addinggolarand@golar/vueto global installs raises the surface area. Consider usingbunxwith--bunflag or a temporary install approach if available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/src/commands/typecheck.ts` around lines 67 - 79, The current branch in typecheck.ts uses isBun to run bun install --global with executorPackages (in the isBun block around the calls to x) which causes unwanted global side effects; change this to avoid global installs by either invoking bunx with the --bun flag or doing a transient/local install instead of --global (e.g., run bunx [executor,...] or install into the local cwd) so you no longer call bun install --global for packages in executorPackages; update the calls around x('bun', [...]) and x('bunx', [...]) to use the non-global approach and ensure nodeOptions/stdio and cwd are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxi/src/commands/typecheck.ts`:
- Line 43: The code silently defaults to 'vue-tsc' when ctx.args.executor is
invalid; update the logic around isExecutor and the executor assignment to
detect an unrecognized ctx.args.executor and either log a warning via the
existing logger/context or throw a user-facing error instead of silently falling
back; refer to isExecutor, ctx.args.executor and the executor variable to
implement a check that emits a clear message like "Unrecognized --executor
value: <value>, defaulting to 'vue-tsc'" (or throws) before setting executor to
'vue-tsc'.
---
Nitpick comments:
In `@packages/nuxi/src/commands/typecheck.ts`:
- Around line 98-100: The isExecutor type guard currently hardcodes 'vue-tsc'
and 'golar'; change it to derive allowed executors from the keys of
executorPackages so it stays in sync: implement isExecutor(value: unknown):
value is Executor by checking typeof value === 'string' and that
(Object.keys(executorPackages) as string[]).includes(value), using
executorPackages (the existing map) and returning that boolean so the guard
reflects the map dynamically.
- Around line 67-79: The current branch in typecheck.ts uses isBun to run bun
install --global with executorPackages (in the isBun block around the calls to
x) which causes unwanted global side effects; change this to avoid global
installs by either invoking bunx with the --bun flag or doing a transient/local
install instead of --global (e.g., run bunx [executor,...] or install into the
local cwd) so you no longer call bun install --global for packages in
executorPackages; update the calls around x('bun', [...]) and x('bunx', [...])
to use the non-global approach and ensure nodeOptions/stdio and cwd are
preserved.
commit: |
1 participant
🔗 Linked issue
❓ Type of change
📚 Description
This PR adds experimental support for
golaras a newtypecheckexecutor innuxi. It introduces executor selection in the command flow, wires package resolution/installation forgolar, and updates the warning message to clearly communicate its experimental status.Below are timing results collected on a private project with more than 7k files.