refactor(appkit): add defineManifest to remove manifest casts - #485
refactor(appkit): add defineManifest to remove manifest casts#485MarioCadenas wants to merge 2 commits into
Conversation
Every plugin declared `static manifest = manifest as PluginManifest` (agents needed `as unknown as`). The cast is unavoidable with a raw JSON import: TS widens JSON fields to `string`, but PluginManifest.resources[].type is the nominal ResourceType enum, so the structural shape never assigns — and multi-resource plugins infer a heterogeneous union that a plain `as` also rejects. Add defineManifest() in the registry: it parses the JSON through the canonical pluginManifestSchema (real runtime validation, which the loader did not do before) and returns the strict type via one audited internal assertion. All 9 plugins now use `static manifest = defineManifest<"name">(manifest)` with no local casts. Export pluginManifestSchema from shared for the parse. Adds tests covering valid pass-through and rejection of unknown resource type, invalid permission, and missing required fields. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
|
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 899 KB (+30 KB) | 311 KB (+8.4 KB) |
| Type declarations | 315 KB (+98 B) | 109 KB (+14 B) |
| Source maps | 1.7 MB (+58 KB) | 580 KB (+14 KB) |
| Other | 11 KB | 3.7 KB |
| Total | 2.9 MB (+88 KB) | 1004 KB (+23 KB) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
93 KB (+4.8 KB) | 2.5 KB | 96 KB (+4.8 KB) | external | 306 KB (+17 KB) |
./beta |
54 KB (+5.1 KB) | 457 B | 54 KB (+5.1 KB) | external | 161 KB (+18 KB) |
./type-generator |
21 KB | 0 B | 21 KB | external | 61 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 89 KB |
. |
utils.js |
initial | 4.0 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 38 KB |
./beta |
stream-manager.js |
initial | 5.8 KB |
./beta |
wide-event-emitter.js |
initial | 3.2 KB |
./beta |
databricks.js |
initial | 3.0 KB |
./beta |
configuration.js |
initial | 2.1 KB |
./beta |
service-context.js |
initial | 1.3 KB |
./beta |
client.js |
initial | 434 B |
./beta |
client-options.js |
initial | 220 B |
./beta |
supervisor-api.js |
lazy | 192 B |
./beta |
databricks.js |
lazy | 142 B |
./beta |
index.js |
lazy | 123 B |
./type-generator |
index.js |
initial | 21 KB |
@databricks/appkit-ui
npm tarball (packed): 342 KB (-285 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 390 KB | 130 KB (+1 B) |
| Type declarations | 228 KB (+32 B) | 83 KB (+8 B) |
| Source maps | 752 KB (-334 B) | 247 KB (-197 B) |
| CSS | 16 KB (-462 B) | 3.2 KB (-90 B) |
| Total | 1.4 MB (-764 B) | 464 KB (-278 B) |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
5.3 KB | 49 KB | 55 KB | 208 KB | 14 KB |
./js/beta |
20 B | 0 B | 20 B | 0 B | 0 B |
./react |
432 KB (+127 B) | 49 KB | 480 KB (+127 B) | 1.3 MB | 175 KB |
./react/beta |
1.0 KB | 0 B | 1.0 KB | 0 B | 1.9 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 5.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 20 B |
./react |
index.js |
initial | 430 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 1.0 KB |
|
This pull request has had no activity for 23 days and has been marked as stale. It will be closed in 7 days if there is no further activity. Add a comment, push a commit, or apply the |
Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com> # Conflicts: # packages/appkit/src/plugins/agents/agents.ts # packages/appkit/src/plugins/analytics/analytics.ts # packages/appkit/src/plugins/files/plugin.ts # packages/appkit/src/plugins/jobs/plugin.ts # packages/appkit/src/plugins/lakebase/lakebase.ts # packages/appkit/src/plugins/vector-search/vector-search.ts # packages/appkit/src/registry/manifest-loader.ts
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 32136855195 -R databricks/appkit -n appkit-template-0.61.1-pr.6740ecc-refactor-define-manifest-485 -D appkit-pr-485 \
&& unzip -o "appkit-pr-485/appkit-template-0.61.1-pr.6740ecc-refactor-define-manifest-485.zip" -d "appkit-pr-485" \
&& databricks apps init --template "appkit-pr-485"The template pins |
What
Introduces
defineManifest()in the registry so plugins stop hand-casting their JSON manifests. All 9 plugins now declare:instead of
static manifest = manifest as PluginManifest<...>(agents needed the uglieras unknown as PluginManifest).Why
The cast was unavoidable with a raw
.jsonimport:string, butPluginManifest.resources[].typeis the nominalResourceTypeenum — a structuralstringnever assigns to it.agentstoday) infer a heterogeneous union forresources, which a plainasalso rejects (hence agents'as unknown as).So the escape hatch wasn't a plugin bug — it was the JSON→type boundary being crossed by assertion in 9 places, with no runtime validation.
How
defineManifest()parses the manifest through the canonicalpluginManifestSchema(Zod) and returns the strict type via one audited internal assertion, after parse has confirmed the values are realResourceType/permission strings. This:<TName>(needed —toPluginderives the typed plugin key frommanifest.name; widening tostringwould collapse the typed registry).pluginManifestSchemais now exported fromsharedfor the parse.Verification
pnpm -r typecheck— clean across all projectsmanifest.jsonfiles through the schema — all OKbiome checkcleanSplit out of the MLflow-tracing PR (
pr/agent-evals-1-tracing), which is where this cast friction first surfaced.