-
-
Notifications
You must be signed in to change notification settings - Fork 377
fix: check if the plugin.constructor exists
#1365
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
WalkthroughOptional chaining added to constructor name checks in src/index.ts to safely access plugin.constructor?.name when detecting 'Elysia' and '_Elysia' within Elysia.use. No other logic or APIs changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
plugin.constructor exists
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/index.ts(2 hunks)
🔇 Additional comments (1)
src/index.ts (1)
4007-4011: Good defensive fix: optional chaining prevents TypeError on Bun 1.2.21.This fallback check is safe and keeps behavior unchanged otherwise.
| if (plugin.constructor?.name === 'Elysia') | ||
| return this._use(plugin.default) | ||
|
|
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.
Bug: checking the module namespace object instead of the default export.
When handling an ES module with a default Elysia instance, this should inspect plugin.default.constructor?.name, not plugin.constructor?.name.
Apply:
- if (plugin.constructor?.name === 'Elysia')
+ if (plugin.default?.constructor?.name === 'Elysia')
return this._use(plugin.default)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (plugin.constructor?.name === 'Elysia') | |
| return this._use(plugin.default) | |
| if (plugin.default?.constructor?.name === 'Elysia') | |
| return this._use(plugin.default) |
🤖 Prompt for AI Agents
In src/index.ts around lines 4018 to 4020, the code is checking
plugin.constructor?.name which inspects the module namespace object for ESM
modules instead of the default export; change the conditional to inspect
plugin.default.constructor?.name and call this._use(plugin.default) for ESM
default exports, while preserving the existing path for non-ESM plugins (i.e.,
first check plugin.constructor?.name === 'Elysia' then fall back to
plugin.default?.constructor?.name === 'Elysia'); also guard against
plugin.default being undefined before accessing its constructor.
| if (plugin.constructor?.name === '_Elysia') | ||
| return this._use(plugin.default) | ||
|
|
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.
Same bug for '_Elysia' branch; also targets the wrong object.
This should also read from plugin.default.
Apply:
- if (plugin.constructor?.name === '_Elysia')
+ if (plugin.default?.constructor?.name === '_Elysia')
return this._use(plugin.default)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (plugin.constructor?.name === '_Elysia') | |
| return this._use(plugin.default) | |
| if (plugin.default?.constructor?.name === '_Elysia') | |
| return this._use(plugin.default) |
🤖 Prompt for AI Agents
In src/index.ts around lines 4021 to 4023, the '_Elysia' branch currently checks
plugin.constructor?.name (the wrong object) and must instead inspect the
exported default; change the condition to read plugin.default?.constructor?.name
=== '_Elysia' and have it call this._use(plugin.default) so both the check and
the argument target the actual default export.
commit: |
Context: oven-sh/bun#22213
Starting from Bun 1.2.21, namespace objects no longer inherit from
Object.prototype(oven-sh/bun#21984). This change was made to comply with the ECMAScript specification.This caused
plugin.constructorto becomeundefinedwhen executingapp.use(import("plugin")), resulting in aTypeErrorbeing thrown when accessingplugin.constructor.name.This PR fixes the issue by using optional chaining to check that
plugin.constructorexists.This change is verified by four existing tests, so no additional tests are included (running the following tests with Bun 1.2.21 will fail):
Summary by CodeRabbit
Bug Fixes
Chores