fix(ai-code-mode-skills): make root export Worker/browser-safe (#486)#736
Open
tombeckenham wants to merge 2 commits into
Conversation
The root entry re-exported `createFileSkillStorage` via `export * from './storage'`, eagerly pulling in `node:fs`/`node:path`. This broke Cloudflare Workers and browser bundlers even for consumers that only used non-storage helpers like `createSkillManagementTools`. The Node-only file storage now lives only behind the `@tanstack/ai-code-mode-skills/storage` subpath. The root re-exports only the browser-safe `createMemorySkillStorage`. Docs, README, JSDoc, and a changeset are updated, plus a regression test that walks the root module graph (static + dynamic imports) and asserts it reaches zero `node:` builtins and that the public export surface is correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR refactors ChangesPackage root export Worker/browser safety
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
🚀 Changeset Version Preview5 package(s) bumped directly, 26 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 4501aff
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/ai-code-mode-skills/README.md`:
- Line 42: The README shows inconsistent return property naming: the usage
example destructures toolsRegistry from codeModeWithSkills but the API Reference
documents the property as registry; update the API Reference to rename the
documented return value from registry to toolsRegistry so the documented shape
of codeModeWithSkills matches the example, and update any parameter/return
descriptions and example snippets that reference registry to use toolsRegistry
instead.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92b279bf-4f04-429d-9414-6d8dc2a1d9e5
📒 Files selected for processing (7)
.changeset/code-mode-skills-worker-safe-root.mddocs/code-mode/code-mode-with-skills.mddocs/config.jsonpackages/ai-code-mode-skills/README.mdpackages/ai-code-mode-skills/src/code-mode-with-skills.tspackages/ai-code-mode-skills/src/index.tspackages/ai-code-mode-skills/tests/root-export-worker-safe.test.ts
|
|
||
| // Build a dynamic registry and system prompt with skills | ||
| const { registry, systemPrompt, selectedSkills } = await codeModeWithSkills({ | ||
| const { toolsRegistry, systemPrompt, selectedSkills } = await codeModeWithSkills({ |
Contributor
There was a problem hiding this comment.
Inconsistent naming between usage example and API reference.
The usage example destructures toolsRegistry (line 42, line 55), but the API Reference section at line 150 (not shown in this diff) still documents the return value as registry. Update the API reference section to match the correct property name toolsRegistry for consistency.
🤖 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/ai-code-mode-skills/README.md` at line 42, The README shows
inconsistent return property naming: the usage example destructures
toolsRegistry from codeModeWithSkills but the API Reference documents the
property as registry; update the API Reference to rename the documented return
value from registry to toolsRegistry so the documented shape of
codeModeWithSkills matches the example, and update any parameter/return
descriptions and example snippets that reference registry to use toolsRegistry
instead.
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.
🎯 Changes
Fixes #486. The
@tanstack/ai-code-mode-skillsroot export was not Worker/browser-safe: it re-exportedcreateFileSkillStorageviaexport * from './storage', which eagerly importsnode:fs/node:path. Cloudflare Workers and browser bundlers failed at build time even for consumers that only used non-storage helpers likecreateSkillManagementToolsorcreateSkillsSystemPrompt.src/index.ts— root entry no longer re-exports the whole./storagebarrel. It re-exports only the browser-safecreateMemorySkillStorage(+MemorySkillStorageOptions). TheSkill/SkillStorage/SkillIndexEntrytypes are unaffected (still exported via the existing./typesblock).createFileSkillStorageis now available only from@tanstack/ai-code-mode-skills/storage.code-mode-with-skills.md), and thecodeModeWithSkillsJSDoc example to import file storage from the subpath, and to document the Worker/browser-safety split. BumpedupdatedAtindocs/config.json.registry→toolsRegistry).from+ dynamicimport()), asserts it reaches zeronode:builtins, and asserts the public export surface (memory storage present, file storage absent at root).Breaking: import
createFileSkillStoragefrom@tanstack/ai-code-mode-skills/storageinstead of the package root.✅ Checklist
pnpm run test:pr.🚀 Release Impact
🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
Improvements
Documentation
Tests