Integrate plugin-hono-server with API Registry protocol#487
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { HonoServerPlugin } from './hono-plugin'; | ||
| import { PluginContext } from '@objectstack/core'; | ||
| import { PluginContext, ApiRegistry } from '@objectstack/core'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, unused imports should be removed to avoid confusion and keep the codebase clean. Here, only ApiRegistry is unused; PluginContext is still used for type casting, so we must preserve it while removing only the unused symbol.
The best minimal fix is to update the import statement on line 3 to remove ApiRegistry and keep PluginContext. No other lines need to change, and this will not modify any runtime behavior because ApiRegistry is not used. The edit is confined to packages/plugins/plugin-hono-server/src/hono-plugin.test.ts, in the import section at the top of the file.
No new methods, imports, or definitions are required; we are only simplifying an existing import.
| @@ -1,6 +1,6 @@ | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { HonoServerPlugin } from './hono-plugin'; | ||
| import { PluginContext, ApiRegistry } from '@objectstack/core'; | ||
| import { PluginContext } from '@objectstack/core'; | ||
|
|
||
| describe('HonoServerPlugin', () => { | ||
| let context: any; |
| @@ -1,10 +1,30 @@ | |||
| import { Plugin, PluginContext, IHttpServer } from '@objectstack/core'; | |||
| import { Plugin, PluginContext, IHttpServer, ApiRegistry } from '@objectstack/core'; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix an unused import, you either remove the import entirely or, if the same import statement brings in multiple symbols, remove just the unused symbol while keeping the others. This improves readability and avoids misleading future maintainers into thinking the symbol is relevant here.
For this specific case, we should edit packages/plugins/plugin-hono-server/src/hono-plugin.ts and adjust the first import line so that it no longer imports IHttpServer. We will keep Plugin, PluginContext, and ApiRegistry unchanged. No new methods, imports, or definitions are required; this is a straightforward edit to the existing import list and does not alter functionality.
Concretely:
- In
packages/plugins/plugin-hono-server/src/hono-plugin.ts, on line 1, change
import { Plugin, PluginContext, IHttpServer, ApiRegistry } from '@objectstack/core';
to
import { Plugin, PluginContext, ApiRegistry } from '@objectstack/core';.
| @@ -1,4 +1,4 @@ | ||
| import { Plugin, PluginContext, IHttpServer, ApiRegistry } from '@objectstack/core'; | ||
| import { Plugin, PluginContext, ApiRegistry } from '@objectstack/core'; | ||
| import { ObjectStackProtocol } from '@objectstack/spec/api'; | ||
| import { | ||
| ApiRegistryEntryInput, |
|
@copilot packages/plugins/plugin-hono-server/src/hono-plugin.ts:566 Error Rule |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request successfully integrates the plugin-hono-server with the API Registry protocol, enabling centralized endpoint management while maintaining full backward compatibility with existing deployments.
Changes:
- Added API Registry integration for centralized endpoint registration and discovery
- Implemented support for REST server configuration via
RestServerConfigfrom the spec - Created a dual-mode system that automatically falls back to legacy route registration when API Registry is unavailable
- Added comprehensive test coverage (9 test cases) covering both new API Registry mode and legacy fallback mode
- Updated documentation with detailed examples and usage patterns for both modes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/plugins/plugin-hono-server/src/hono-plugin.ts | Core implementation: added API Registry integration, REST config support, dual-mode routing (registry vs legacy), extracted helper methods and constants, fixed HTTP 304 compliance |
| packages/plugins/plugin-hono-server/src/hono-plugin.test.ts | Comprehensive test suite covering API Registry integration, configuration options, fallback behavior, and error handling |
| packages/plugins/plugin-hono-server/README.md | Updated documentation with API Registry usage examples, configuration options, migration guide, and extensibility patterns |
| } | ||
| ``` | ||
|
|
||
| **Note:** The Hono Server Plugin loads routes from the API Registry sorted by priority (highest first), ensuring core endpoints take precedence over plugin endpoints. |
There was a problem hiding this comment.
The note states "The Hono Server Plugin loads routes from the API Registry sorted by priority" but this is misleading. Based on the implementation in bindEndpoint() (hono-plugin.ts:586-589), the plugin only successfully binds routes for which it has handlers defined in createHandlerForEndpoint(). For custom plugin endpoints registered in the API Registry, bindEndpoint() will log a warning "No handler found for endpoint" and skip binding them.
Consider clarifying this note to explain that:
- HonoServerPlugin only auto-binds standard ObjectStack endpoints from the registry
- Custom plugins must bind their own handlers separately via the
http-serverservice (as shown in the example above at lines 329-336) - The API Registry serves primarily for endpoint discovery and documentation, not automatic handler binding for plugin endpoints
This would help developers understand that registering an endpoint in the API Registry does not automatically make it functional - they must also bind the handler implementation.
| **Note:** The Hono Server Plugin loads routes from the API Registry sorted by priority (highest first), ensuring core endpoints take precedence over plugin endpoints. | |
| **Note:** The Hono Server Plugin loads **only the standard ObjectStack endpoints it knows how to handle** from the API Registry, and processes them sorted by priority (highest first) so core endpoints take precedence. Custom plugin endpoints registered in the API Registry are **not** automatically bound; they must still bind their own handlers via the `http-server` service (as shown in the `start()` example above). For plugin-defined APIs, the API Registry primarily serves for endpoint discovery and documentation rather than automatic handler binding. |
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { HonoServerPlugin } from './hono-plugin'; | ||
| import { PluginContext } from '@objectstack/core'; | ||
| import { PluginContext, ApiRegistry } from '@objectstack/core'; |
There was a problem hiding this comment.
Unused import ApiRegistry.
| import { PluginContext, ApiRegistry } from '@objectstack/core'; | |
| import { PluginContext } from '@objectstack/core'; |
| @@ -1,10 +1,30 @@ | |||
| import { Plugin, PluginContext, IHttpServer } from '@objectstack/core'; | |||
| import { Plugin, PluginContext, IHttpServer, ApiRegistry } from '@objectstack/core'; | |||
There was a problem hiding this comment.
Unused import IHttpServer.
| import { Plugin, PluginContext, IHttpServer, ApiRegistry } from '@objectstack/core'; | |
| import { Plugin, PluginContext, ApiRegistry } from '@objectstack/core'; |
Transform plugin-hono-server to use latest API protocol ✅
All Changes Completed Successfully! 🎉
rest-server.zod.tsSummary:
Successfully transformed the plugin-hono-server to use the latest API protocol while maintaining 100% backward compatibility. The plugin now supports:
✅ API Registry Integration - Centralized endpoint management
✅ REST Server Configuration - Following
rest-server.zod.tsprotocol✅ Dynamic Route Loading - Priority-based route binding from registry
✅ Plugin Extensibility - Custom endpoint registration support
✅ Legacy Fallback - Automatic compatibility mode when registry unavailable
✅ Clean Code - Constants extracted, duplication reduced, HTTP spec compliant
✅ Comprehensive Tests - 9/9 tests passing
✅ Complete Documentation - README updated with examples and migration guide
✅ CodeQL Clean - All static analysis warnings resolved
Code Quality:
DEFAULT_ENDPOINT_PRIORITY,CORE_ENDPOINT_PRIORITY,DISCOVERY_ENDPOINT_PRIORITYcreateCacheRequest()to reduce duplicationOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.