-
-
Notifications
You must be signed in to change notification settings - Fork 472
fix(ui/cli/plugins): setup-plugin-bun
also detect Bun.build
#1615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 9336ab1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Caution Review failedThe pull request is closed. WalkthroughThe updateBuildConfig utility now recognizes Bun.build() calls in addition to build() when injecting a plugin and import. A new unit test covers Bun.build-style configs. A changeset marks a patch release noting Bun.build detection. The Bun docs guide now uses a non-interactive Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as updateBuildConfig
participant Parser as AST Parser
participant File as build.config.ts
Dev->>CLI: run updateBuildConfig(File, pluginName, importPath)
CLI->>Parser: parse File to AST
Parser-->>CLI: AST
CLI->>CLI: detect call to build() or Bun.build()
alt Detected
CLI->>CLI: ensure import for plugin is present
CLI->>CLI: ensure plugins array includes plugin (de-duplicated)
CLI->>File: write updated config
else Not detected
CLI-->>Dev: no changes
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/many-nights-nail.md (1)
5-5
: Polish wording for clarity/tenseConsider tightening the message:
- Current: fix(ui/cli/plugins):
setup-plugin-bun
also detectBun.build
- Suggested: fix(ui/cli/plugins): setup-plugin-bun also detects Bun.build()
-fix(ui/cli/plugins): `setup-plugin-bun` also detect `Bun.build` +fix(ui/cli/plugins): setup-plugin-bun also detects Bun.build()packages/ui/src/cli/utils/update-build-config.ts (1)
43-51
: Harden callee detection: computed access and optional chaining (Bun['build'], Bun?.build)Edge cases not currently recognized:
- Bun"build"
- Bun?.build(...)
Also, plain Identifier build(...) could match non-Bun functions in rare cases. If you want to be stricter, confirm build is imported from "bun" (including alias).
Minimal change within this block to support computed access and optional chaining:
- // Check if this is a build() or Bun.build() call - if ( - ((node.callee.type === "Identifier" && node.callee.name === "build") || - (node.callee.type === "MemberExpression" && - node.callee.object.type === "Identifier" && - node.callee.object.name === "Bun" && - node.callee.property.type === "Identifier" && - node.callee.property.name === "build")) && + // Check if this is a build() or Bun.build()/Bun["build"] call (incl. optional chaining) + const callee = + node.callee.type === "ChainExpression" ? node.callee.expression : node.callee; + if ( + ((callee.type === "Identifier" && callee.name === "build") || + (callee.type === "MemberExpression" && + callee.object.type === "Identifier" && + callee.object.name === "Bun" && + ( + (callee.property.type === "Identifier" && callee.property.name === "build") || + (callee.property.type === "Literal" && callee.property.value === "build") + ))) && node.arguments.length > 0 && node.arguments[0].type === "ObjectExpression" ) {Follow-up (optional): cross-check that an Identifier build comes from import ... from "bun" (and support alias import) to reduce false positives. I can sketch that if useful.
packages/ui/src/cli/utils/update-build-config.test.ts (1)
94-120
: Add a couple more cases to harden behavior (optional)Consider adding:
- Bun"build" to cover computed access.
- Alias import: import { build as bunBuild } from "bun"; await bunBuild({...}); to ensure Identifier checks work with aliases (if/when supported in code).
Example test sketch:
it("should handle config with Bun['build']", () => { const input = ` const result = await Bun["build"]({ plugins: [], }); `; const result = updateBuildConfig({ content: input, pluginName: "flowbiteReact", pluginImportPath: "flowbite-react/plugin/bun", }); expect(result).toContain('import flowbiteReact from "flowbite-react/plugin/bun"'); expect(result).toMatch(/plugins:\s*\[\s*flowbiteReact\s*\]/); }); it("should handle aliased build import from bun", () => { const input = ` import { build as bunBuild } from "bun"; const result = await bunBuild({ plugins: [] }); `; const result = updateBuildConfig({ content: input, pluginName: "flowbiteReact", pluginImportPath: "flowbite-react/plugin/test", }); expect(result).toContain('import flowbiteReact from "flowbite-react/plugin/test"'); expect(result).toMatch(/plugins:\s*\[\s*flowbiteReact\s*\]/); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/many-nights-nail.md
(1 hunks)packages/ui/src/cli/utils/update-build-config.test.ts
(1 hunks)packages/ui/src/cli/utils/update-build-config.ts
(1 hunks)
🔇 Additional comments (2)
packages/ui/src/cli/utils/update-build-config.ts (1)
43-51
: LGTM: Bun.build() detection added correctlyThe callee check now handles both build(...) and Bun.build(...). This aligns with the PR goal and keeps downstream plugin insertion unchanged.
packages/ui/src/cli/utils/update-build-config.test.ts (1)
94-120
: Good coverage for Bun.build() pathThe test validates both the added import and plugin injection order with Bun.build. Matches the updated logic.
… React and Tailwind CSS integration
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chores