diff --git a/apps/dev-playground/client/src/appkit-types/analytics.d.ts b/apps/dev-playground/client/src/appkit-types/analytics.d.ts index 0e0ae0b0..43666dd0 100644 --- a/apps/dev-playground/client/src/appkit-types/analytics.d.ts +++ b/apps/dev-playground/client/src/appkit-types/analytics.d.ts @@ -119,10 +119,10 @@ declare module "@databricks/appkit-ui/react" { result: Array<{ /** @sqlType STRING */ string_value: string; - /** @sqlType STRING */ - number_value: string; - /** @sqlType STRING */ - boolean_value: string; + /** @sqlType INT */ + number_value: number; + /** @sqlType BOOLEAN */ + boolean_value: boolean; /** @sqlType STRING */ date_value: string; /** @sqlType STRING */ diff --git a/apps/dev-playground/client/src/routeTree.gen.ts b/apps/dev-playground/client/src/routeTree.gen.ts index 99ac75fc..4a9f72fa 100644 --- a/apps/dev-playground/client/src/routeTree.gen.ts +++ b/apps/dev-playground/client/src/routeTree.gen.ts @@ -9,6 +9,7 @@ // Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified. import { Route as rootRouteImport } from './routes/__root' +import { Route as VectorSearchRouteRouteImport } from './routes/vector-search.route' import { Route as TypeSafetyRouteRouteImport } from './routes/type-safety.route' import { Route as TelemetryRouteRouteImport } from './routes/telemetry.route' import { Route as SqlHelpersRouteRouteImport } from './routes/sql-helpers.route' @@ -23,6 +24,11 @@ import { Route as ArrowAnalyticsRouteRouteImport } from './routes/arrow-analytic import { Route as AnalyticsRouteRouteImport } from './routes/analytics.route' import { Route as IndexRouteImport } from './routes/index' +const VectorSearchRouteRoute = VectorSearchRouteRouteImport.update({ + id: '/vector-search', + path: '/vector-search', + getParentRoute: () => rootRouteImport, +} as any) const TypeSafetyRouteRoute = TypeSafetyRouteRouteImport.update({ id: '/type-safety', path: '/type-safety', @@ -103,6 +109,7 @@ export interface FileRoutesByFullPath { '/sql-helpers': typeof SqlHelpersRouteRoute '/telemetry': typeof TelemetryRouteRoute '/type-safety': typeof TypeSafetyRouteRoute + '/vector-search': typeof VectorSearchRouteRoute } export interface FileRoutesByTo { '/': typeof IndexRoute @@ -118,6 +125,7 @@ export interface FileRoutesByTo { '/sql-helpers': typeof SqlHelpersRouteRoute '/telemetry': typeof TelemetryRouteRoute '/type-safety': typeof TypeSafetyRouteRoute + '/vector-search': typeof VectorSearchRouteRoute } export interface FileRoutesById { __root__: typeof rootRouteImport @@ -134,6 +142,7 @@ export interface FileRoutesById { '/sql-helpers': typeof SqlHelpersRouteRoute '/telemetry': typeof TelemetryRouteRoute '/type-safety': typeof TypeSafetyRouteRoute + '/vector-search': typeof VectorSearchRouteRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath @@ -151,6 +160,7 @@ export interface FileRouteTypes { | '/sql-helpers' | '/telemetry' | '/type-safety' + | '/vector-search' fileRoutesByTo: FileRoutesByTo to: | '/' @@ -166,6 +176,7 @@ export interface FileRouteTypes { | '/sql-helpers' | '/telemetry' | '/type-safety' + | '/vector-search' id: | '__root__' | '/' @@ -181,6 +192,7 @@ export interface FileRouteTypes { | '/sql-helpers' | '/telemetry' | '/type-safety' + | '/vector-search' fileRoutesById: FileRoutesById } export interface RootRouteChildren { @@ -197,10 +209,18 @@ export interface RootRouteChildren { SqlHelpersRouteRoute: typeof SqlHelpersRouteRoute TelemetryRouteRoute: typeof TelemetryRouteRoute TypeSafetyRouteRoute: typeof TypeSafetyRouteRoute + VectorSearchRouteRoute: typeof VectorSearchRouteRoute } declare module '@tanstack/react-router' { interface FileRoutesByPath { + '/vector-search': { + id: '/vector-search' + path: '/vector-search' + fullPath: '/vector-search' + preLoaderRoute: typeof VectorSearchRouteRouteImport + parentRoute: typeof rootRouteImport + } '/type-safety': { id: '/type-safety' path: '/type-safety' @@ -309,6 +329,7 @@ const rootRouteChildren: RootRouteChildren = { SqlHelpersRouteRoute: SqlHelpersRouteRoute, TelemetryRouteRoute: TelemetryRouteRoute, TypeSafetyRouteRoute: TypeSafetyRouteRoute, + VectorSearchRouteRoute: VectorSearchRouteRoute, } export const routeTree = rootRouteImport ._addFileChildren(rootRouteChildren) diff --git a/apps/dev-playground/server/index.ts b/apps/dev-playground/server/index.ts index 913f547c..87dbf7d2 100644 --- a/apps/dev-playground/server/index.ts +++ b/apps/dev-playground/server/index.ts @@ -26,7 +26,7 @@ function createMockClient() { createApp({ plugins: [ - server({ autoStart: false }), + server(), reconnect(), telemetryExamples(), analytics({}), @@ -49,9 +49,8 @@ createApp({ // }), ], ...(process.env.APPKIT_E2E_TEST && { client: createMockClient() }), -}).then((appkit) => { - appkit.server - .extend((app) => { + onPluginsReady(appkit) { + appkit.server.extend((app) => { app.get("/sp", (_req, res) => { appkit.analytics .query("SELECT * FROM samples.nyctaxi.trips;") @@ -86,6 +85,6 @@ createApp({ }); }); }); - }) - .start(); -}); + }); + }, +}).catch(console.error); diff --git a/docs/docs/api/appkit/Class.ServerError.md b/docs/docs/api/appkit/Class.ServerError.md index d3dce68e..cce86cad 100644 --- a/docs/docs/api/appkit/Class.ServerError.md +++ b/docs/docs/api/appkit/Class.ServerError.md @@ -6,7 +6,6 @@ Use for server start/stop issues, configuration conflicts, etc. ## Example ```typescript -throw new ServerError("Cannot get server when autoStart is true"); throw new ServerError("Server not started"); ``` @@ -151,26 +150,6 @@ Create a human-readable string representation *** -### autoStartConflict() - -```ts -static autoStartConflict(operation: string): ServerError; -``` - -Create a server error for autoStart conflict - -#### Parameters - -| Parameter | Type | -| ------ | ------ | -| `operation` | `string` | - -#### Returns - -`ServerError` - -*** - ### clientDirectoryNotFound() ```ts diff --git a/docs/docs/api/appkit/Function.createApp.md b/docs/docs/api/appkit/Function.createApp.md index cb703386..6a0b7cb2 100644 --- a/docs/docs/api/appkit/Function.createApp.md +++ b/docs/docs/api/appkit/Function.createApp.md @@ -4,6 +4,7 @@ function createApp(config: { cache?: CacheConfig; client?: WorkspaceClient; + onPluginsReady?: (appkit: PluginMap) => void | Promise; plugins?: T; telemetry?: TelemetryConfig; }): Promise>; @@ -13,6 +14,9 @@ Bootstraps AppKit with the provided configuration. Initializes telemetry, cache, and service context, then registers plugins in phase order (core, normal, deferred) and awaits their setup. +If a `onPluginsReady` callback is provided it runs after plugin setup but +before the server starts, giving you access to the full appkit handle +for registering custom routes or performing async setup. The returned object maps each plugin name to its `exports()` API, with an `asUser(req)` method for user-scoped execution. @@ -26,9 +30,10 @@ with an `asUser(req)` method for user-scoped execution. | Parameter | Type | | ------ | ------ | -| `config` | \{ `cache?`: [`CacheConfig`](Interface.CacheConfig.md); `client?`: `WorkspaceClient`; `plugins?`: `T`; `telemetry?`: [`TelemetryConfig`](Interface.TelemetryConfig.md); \} | +| `config` | \{ `cache?`: [`CacheConfig`](Interface.CacheConfig.md); `client?`: `WorkspaceClient`; `onPluginsReady?`: (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\>; `plugins?`: `T`; `telemetry?`: [`TelemetryConfig`](Interface.TelemetryConfig.md); \} | | `config.cache?` | [`CacheConfig`](Interface.CacheConfig.md) | | `config.client?` | `WorkspaceClient` | +| `config.onPluginsReady?` | (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\> | | `config.plugins?` | `T` | | `config.telemetry?` | [`TelemetryConfig`](Interface.TelemetryConfig.md) | @@ -51,12 +56,12 @@ await createApp({ ```ts import { createApp, server, analytics } from "@databricks/appkit"; -const appkit = await createApp({ - plugins: [server({ autoStart: false }), analytics({})], -}); - -appkit.server.extend((app) => { - app.get("/custom", (_req, res) => res.json({ ok: true })); +await createApp({ + plugins: [server(), analytics({})], + onPluginsReady(appkit) { + appkit.server.extend((app) => { + app.get("/custom", (_req, res) => res.json({ ok: true })); + }); + }, }); -await appkit.server.start(); ``` diff --git a/docs/docs/plugins/server.md b/docs/docs/plugins/server.md index 389828dc..6cfaa7b7 100644 --- a/docs/docs/plugins/server.md +++ b/docs/docs/plugins/server.md @@ -36,22 +36,38 @@ await createApp({ }); ``` -## Manual server start example +## Custom routes example -When you need to extend Express with custom routes: +Use the `onPluginsReady` callback to extend Express with custom routes before the server starts: ```ts import { createApp, server } from "@databricks/appkit"; -const appkit = await createApp({ - plugins: [server({ autoStart: false })], +await createApp({ + plugins: [server()], + onPluginsReady(appkit) { + appkit.server.extend((app) => { + app.get("/custom", (_req, res) => res.json({ ok: true })); + }); + }, }); +``` -appkit.server.extend((app) => { - app.get("/custom", (_req, res) => res.json({ ok: true })); -}); +The `onPluginsReady` callback also supports async operations: -await appkit.server.start(); +```ts +await createApp({ + plugins: [server()], + async onPluginsReady(appkit) { + const pool = await initializeDatabase(); + appkit.server.extend((app) => { + app.get("/data", async (_req, res) => { + const result = await pool.query("SELECT 1"); + res.json(result); + }); + }); + }, +}); ``` ## Configuration options @@ -64,7 +80,6 @@ await createApp({ server({ port: 8000, // default: Number(process.env.DATABRICKS_APP_PORT) || 8000 host: "0.0.0.0", // default: process.env.FLASK_RUN_HOST || "0.0.0.0" - autoStart: true, // default: true staticPath: "dist", // optional: force a specific static directory }), ], diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index a2cba994..40fd1fcd 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -167,6 +167,7 @@ export class AppKit { telemetry?: TelemetryConfig; cache?: CacheConfig; client?: WorkspaceClient; + onPluginsReady?: (appkit: PluginMap) => void | Promise; } = {}, ): Promise> { // Initialize core services @@ -200,7 +201,16 @@ export class AppKit { await Promise.all(instance.#setupPromises); - return instance as unknown as PluginMap; + const handle = instance as unknown as PluginMap; + + await config.onPluginsReady?.(handle); + + const serverPlugin = instance.#pluginInstances.server; + if (serverPlugin && typeof (serverPlugin as any).start === "function") { + await (serverPlugin as any).start(); + } + + return handle; } private static preparePlugins( @@ -222,6 +232,9 @@ export class AppKit { * * Initializes telemetry, cache, and service context, then registers plugins * in phase order (core, normal, deferred) and awaits their setup. + * If a `onPluginsReady` callback is provided it runs after plugin setup but + * before the server starts, giving you access to the full appkit handle + * for registering custom routes or performing async setup. * The returned object maps each plugin name to its `exports()` API, * with an `asUser(req)` method for user-scoped execution. * @@ -236,18 +249,18 @@ export class AppKit { * }); * ``` * - * @example Extended Server with analytics and custom endpoint + * @example Server with custom routes via onPluginsReady * ```ts * import { createApp, server, analytics } from "@databricks/appkit"; * - * const appkit = await createApp({ - * plugins: [server({ autoStart: false }), analytics({})], - * }); - * - * appkit.server.extend((app) => { - * app.get("/custom", (_req, res) => res.json({ ok: true })); + * await createApp({ + * plugins: [server(), analytics({})], + * onPluginsReady(appkit) { + * appkit.server.extend((app) => { + * app.get("/custom", (_req, res) => res.json({ ok: true })); + * }); + * }, * }); - * await appkit.server.start(); * ``` */ export async function createApp< @@ -258,6 +271,7 @@ export async function createApp< telemetry?: TelemetryConfig; cache?: CacheConfig; client?: WorkspaceClient; + onPluginsReady?: (appkit: PluginMap) => void | Promise; } = {}, ): Promise> { return AppKit._createApp(config); diff --git a/packages/appkit/src/errors/server.ts b/packages/appkit/src/errors/server.ts index 6af5b59f..d45148d8 100644 --- a/packages/appkit/src/errors/server.ts +++ b/packages/appkit/src/errors/server.ts @@ -6,7 +6,6 @@ import { AppKitError } from "./base"; * * @example * ```typescript - * throw new ServerError("Cannot get server when autoStart is true"); * throw new ServerError("Server not started"); * ``` */ @@ -15,15 +14,6 @@ export class ServerError extends AppKitError { readonly statusCode = 500; readonly isRetryable = false; - /** - * Create a server error for autoStart conflict - */ - static autoStartConflict(operation: string): ServerError { - return new ServerError(`Cannot ${operation} when autoStart is true`, { - context: { operation }, - }); - } - /** * Create a server error for server not started */ diff --git a/packages/appkit/src/errors/tests/errors.test.ts b/packages/appkit/src/errors/tests/errors.test.ts index c404a18f..347ce1c0 100644 --- a/packages/appkit/src/errors/tests/errors.test.ts +++ b/packages/appkit/src/errors/tests/errors.test.ts @@ -348,12 +348,6 @@ describe("ServerError", () => { expect(error.isRetryable).toBe(false); }); - test("autoStartConflict should create proper error", () => { - const error = ServerError.autoStartConflict("get server"); - expect(error.message).toBe("Cannot get server when autoStart is true"); - expect(error.context?.operation).toBe("get server"); - }); - test("notStarted should create proper error", () => { const error = ServerError.notStarted(); expect(error.message).toContain("Server not started"); diff --git a/packages/appkit/src/plugins/analytics/tests/analytics.integration.test.ts b/packages/appkit/src/plugins/analytics/tests/analytics.integration.test.ts index cb73394a..0cec2298 100644 --- a/packages/appkit/src/plugins/analytics/tests/analytics.integration.test.ts +++ b/packages/appkit/src/plugins/analytics/tests/analytics.integration.test.ts @@ -46,13 +46,11 @@ describe("Analytics Plugin Integration", () => { serverPlugin({ port: TEST_PORT, host: "127.0.0.1", - autoStart: false, }), analytics({}), ], }); - await app.server.start(); server = app.server.getServer(); baseUrl = `http://127.0.0.1:${TEST_PORT}`; }); diff --git a/packages/appkit/src/plugins/files/tests/plugin.integration.test.ts b/packages/appkit/src/plugins/files/tests/plugin.integration.test.ts index 55989148..add134d7 100644 --- a/packages/appkit/src/plugins/files/tests/plugin.integration.test.ts +++ b/packages/appkit/src/plugins/files/tests/plugin.integration.test.ts @@ -87,13 +87,11 @@ describe("Files Plugin Integration", () => { serverPlugin({ port: TEST_PORT, host: "127.0.0.1", - autoStart: false, }), files(), ], }); - await appkit.server.start(); server = appkit.server.getServer(); baseUrl = `http://127.0.0.1:${TEST_PORT}`; }); diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index e7b9b31a..6c484114 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -27,17 +27,23 @@ const logger = createLogger("server"); * This plugin is responsible for starting the server and serving the static files. * It also handles the remote tunneling for development purposes. * + * The server is started automatically by `createApp` after all plugins are set up + * and the optional `onPluginsReady` callback has run. + * * @example * ```ts * createApp({ - * plugins: [server(), telemetryExamples(), analytics({})], + * plugins: [server(), analytics({})], + * onPluginsReady(appkit) { + * appkit.server.extend((app) => { + * app.get("/custom", (_req, res) => res.json({ ok: true })); + * }); + * }, * }); * ``` - * */ export class ServerPlugin extends Plugin { public static DEFAULT_CONFIG = { - autoStart: true, host: process.env.FLASK_RUN_HOST || "0.0.0.0", port: Number(process.env.DATABRICKS_APP_PORT) || 8000, }; @@ -54,6 +60,13 @@ export class ServerPlugin extends Plugin { static phase: PluginPhase = "deferred"; constructor(config: ServerConfig) { + if ("autoStart" in config) { + throw new ServerError( + "server({ autoStart }) has been removed. " + + "The server is now started automatically by createApp.\n\n" + + "Run `npx appkit codemod customize-callback --write` to auto-migrate.", + ); + } super(config); this.config = config; this.serverApplication = express(); @@ -65,12 +78,7 @@ export class ServerPlugin extends Plugin { ]); } - /** Setup the server plugin. */ - async setup() { - if (this.shouldAutoStart()) { - await this.start(); - } - } + async setup() {} /** Get the server configuration. */ getConfig() { @@ -79,11 +87,6 @@ export class ServerPlugin extends Plugin { return config; } - /** Check if the server should auto start. */ - shouldAutoStart() { - return this.config.autoStart; - } - /** * Start the server. * @@ -148,14 +151,10 @@ export class ServerPlugin extends Plugin { * * Only use this method if you need to access the server instance for advanced usage like a custom websocket server, etc. * - * @throws {Error} If the server is not started or autoStart is true. + * @throws {Error} If the server has not started yet. * @returns {HTTPServer} The server instance. */ getServer(): HTTPServer { - if (this.shouldAutoStart()) { - throw ServerError.autoStartConflict("get server"); - } - if (!this.server) { throw ServerError.notStarted(); } @@ -166,15 +165,13 @@ export class ServerPlugin extends Plugin { /** * Extend the server with custom routes or middleware. * + * Call this inside the `onPluginsReady` callback of `createApp` to register + * custom Express routes or middleware before the server starts listening. + * * @param fn - A function that receives the express application. * @returns The server plugin instance for chaining. - * @throws {Error} If autoStart is true. */ extend(fn: (app: express.Application) => void) { - if (this.shouldAutoStart()) { - throw ServerError.autoStartConflict("extend server"); - } - this.serverExtensions.push(fn); return this; } @@ -389,8 +386,6 @@ export class ServerPlugin extends Plugin { exports() { const self = this; return { - /** Start the server */ - start: this.start, /** Extend the server with custom routes or middleware */ extend(fn: (app: express.Application) => void) { self.extend(fn); @@ -400,6 +395,19 @@ export class ServerPlugin extends Plugin { getServer: this.getServer, /** Get the server configuration */ getConfig: this.getConfig, + /** @deprecated Server is now started automatically by createApp. */ + start() { + throw new ServerError( + "server.start() has been removed. Use the onPluginsReady callback instead:\n\n" + + " createApp({\n" + + " plugins: [server(), ...],\n" + + " onPluginsReady(appkit) {\n" + + " appkit.server.extend(...);\n" + + " },\n" + + " });\n\n" + + "Run `npx appkit codemod customize-callback --write` to auto-migrate.", + ); + }, }; } } diff --git a/packages/appkit/src/plugins/server/manifest.json b/packages/appkit/src/plugins/server/manifest.json index 11822beb..1112fbf5 100644 --- a/packages/appkit/src/plugins/server/manifest.json +++ b/packages/appkit/src/plugins/server/manifest.json @@ -11,11 +11,6 @@ "schema": { "type": "object", "properties": { - "autoStart": { - "type": "boolean", - "default": true, - "description": "Automatically start the server on plugin setup" - }, "host": { "type": "string", "default": "0.0.0.0", diff --git a/packages/appkit/src/plugins/server/tests/server.integration.test.ts b/packages/appkit/src/plugins/server/tests/server.integration.test.ts index c3a646ea..0b67e4c0 100644 --- a/packages/appkit/src/plugins/server/tests/server.integration.test.ts +++ b/packages/appkit/src/plugins/server/tests/server.integration.test.ts @@ -29,13 +29,10 @@ describe("ServerPlugin Integration", () => { serverPlugin({ port: TEST_PORT, host: "127.0.0.1", - autoStart: false, }), ], }); - // Start server manually - await app.server.start(); server = app.server.getServer(); baseUrl = `http://127.0.0.1:${TEST_PORT}`; @@ -124,13 +121,11 @@ describe("ServerPlugin with custom plugin", () => { serverPlugin({ port: TEST_PORT, host: "127.0.0.1", - autoStart: false, }), testPlugin({}), ], }); - await app.server.start(); server = app.server.getServer(); baseUrl = `http://127.0.0.1:${TEST_PORT}`; @@ -172,7 +167,7 @@ describe("ServerPlugin with custom plugin", () => { }); }); -describe("ServerPlugin with extend()", () => { +describe("ServerPlugin with extend() via onPluginsReady", () => { let server: Server; let baseUrl: string; let serviceContextMock: Awaited>; @@ -188,19 +183,73 @@ describe("ServerPlugin with extend()", () => { serverPlugin({ port: TEST_PORT, host: "127.0.0.1", - autoStart: false, }), ], + onPluginsReady(appkit) { + appkit.server.extend((expressApp) => { + expressApp.get("/custom", (_req, res) => { + res.json({ custom: true }); + }); + }); + }, }); - // Add custom route via extend() - app.server.extend((expressApp) => { - expressApp.get("/custom", (_req, res) => { - res.json({ custom: true }); + server = app.server.getServer(); + baseUrl = `http://127.0.0.1:${TEST_PORT}`; + + await new Promise((resolve) => setTimeout(resolve, 100)); + }); + + afterAll(async () => { + serviceContextMock?.restore(); + if (server) { + await new Promise((resolve, reject) => { + server.close((err) => { + if (err) reject(err); + else resolve(); + }); }); + } + }); + + test("custom route via extend() in onPluginsReady callback works", async () => { + const response = await fetch(`${baseUrl}/custom`); + + expect(response.status).toBe(200); + + const data = await response.json(); + expect(data).toEqual({ custom: true }); + }); +}); + +describe("createApp with async onPluginsReady callback", () => { + let server: Server; + let baseUrl: string; + let serviceContextMock: Awaited>; + const TEST_PORT = 9885; + + beforeAll(async () => { + setupDatabricksEnv(); + ServiceContext.reset(); + serviceContextMock = await mockServiceContext(); + + const app = await createApp({ + plugins: [ + serverPlugin({ + port: TEST_PORT, + host: "127.0.0.1", + }), + ], + async onPluginsReady(appkit) { + await new Promise((resolve) => setTimeout(resolve, 10)); + appkit.server.extend((expressApp) => { + expressApp.get("/async-custom", (_req, res) => { + res.json({ asyncSetup: true }); + }); + }); + }, }); - await app.server.start(); server = app.server.getServer(); baseUrl = `http://127.0.0.1:${TEST_PORT}`; @@ -219,12 +268,38 @@ describe("ServerPlugin with extend()", () => { } }); - test("custom route via extend() works", async () => { - const response = await fetch(`${baseUrl}/custom`); + test("async onPluginsReady callback runs before server starts", async () => { + const response = await fetch(`${baseUrl}/async-custom`); expect(response.status).toBe(200); const data = await response.json(); - expect(data).toEqual({ custom: true }); + expect(data).toEqual({ asyncSetup: true }); + }); +}); + +describe("createApp without server plugin", () => { + let serviceContextMock: Awaited>; + let onPluginsReadyWasCalled = false; + + beforeAll(async () => { + setupDatabricksEnv(); + ServiceContext.reset(); + serviceContextMock = await mockServiceContext(); + + await createApp({ + plugins: [], + onPluginsReady() { + onPluginsReadyWasCalled = true; + }, + }); + }); + + afterAll(async () => { + serviceContextMock?.restore(); + }); + + test("onPluginsReady callback is still called without server plugin", () => { + expect(onPluginsReadyWasCalled).toBe(true); }); }); diff --git a/packages/appkit/src/plugins/server/tests/server.test.ts b/packages/appkit/src/plugins/server/tests/server.test.ts index 22f18129..fae11fb5 100644 --- a/packages/appkit/src/plugins/server/tests/server.test.ts +++ b/packages/appkit/src/plugins/server/tests/server.test.ts @@ -197,19 +197,22 @@ describe("ServerPlugin", () => { const plugin = new ServerPlugin({ port: 3000, host: "127.0.0.1", - autoStart: false, }); const config = plugin.getConfig(); expect(config.port).toBe(3000); expect(config.host).toBe("127.0.0.1"); - expect(config.autoStart).toBe(false); + }); + + test("should throw when autoStart is passed", () => { + expect(() => new ServerPlugin({ autoStart: false } as any)).toThrow( + "server({ autoStart }) has been removed", + ); }); }); describe("DEFAULT_CONFIG", () => { test("should have correct default values", () => { - expect(ServerPlugin.DEFAULT_CONFIG.autoStart).toBe(true); expect(ServerPlugin.DEFAULT_CONFIG.host).toBe("0.0.0.0"); expect(ServerPlugin.DEFAULT_CONFIG.port).toBe(8000); }); @@ -220,30 +223,9 @@ describe("ServerPlugin", () => { }); }); - describe("shouldAutoStart", () => { - test("should return true when autoStart is true", () => { - const plugin = new ServerPlugin({ autoStart: true }); - expect(plugin.shouldAutoStart()).toBe(true); - }); - - test("should return false when autoStart is false", () => { - const plugin = new ServerPlugin({ autoStart: false }); - expect(plugin.shouldAutoStart()).toBe(false); - }); - }); - describe("setup", () => { - test("should call start when autoStart is true", async () => { - const plugin = new ServerPlugin({ autoStart: true }); - const startSpy = vi.spyOn(plugin, "start").mockResolvedValue({} as any); - - await plugin.setup(); - - expect(startSpy).toHaveBeenCalled(); - }); - - test("should not call start when autoStart is false", async () => { - const plugin = new ServerPlugin({ autoStart: false }); + test("should be a no-op (server start is orchestrated by createApp)", async () => { + const plugin = new ServerPlugin({}); const startSpy = vi.spyOn(plugin, "start").mockResolvedValue({} as any); await plugin.setup(); @@ -254,7 +236,7 @@ describe("ServerPlugin", () => { describe("start", () => { test("should call listen on express app", async () => { - const plugin = new ServerPlugin({ autoStart: false, port: 3000 }); + const plugin = new ServerPlugin({ port: 3000 }); await plugin.start(); @@ -267,7 +249,7 @@ describe("ServerPlugin", () => { test("should setup ViteDevServer in development mode", async () => { process.env.NODE_ENV = "development"; - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); await plugin.start(); @@ -277,7 +259,7 @@ describe("ServerPlugin", () => { }); test("should register RemoteTunnelController middleware and set server", async () => { - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); await plugin.start(); @@ -304,7 +286,7 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ plugins }); await plugin.start(); // Get the type function passed to express.json @@ -348,7 +330,7 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ plugins }); await plugin.start(); const routerFn = (express as any).Router as ReturnType; @@ -386,7 +368,7 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ plugins }); await plugin.start(); expect(plugins["plugin-a"].clientConfig).toHaveBeenCalled(); @@ -413,7 +395,7 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ plugins }); await plugin.start(); expect(plugins["plugin-null"].clientConfig).toHaveBeenCalled(); @@ -444,7 +426,7 @@ describe("ServerPlugin", () => { }, }; - const plugin = new ServerPlugin({ autoStart: false, plugins }); + const plugin = new ServerPlugin({ plugins }); await expect(plugin.start()).resolves.toBeDefined(); expect(mockLoggerError).toHaveBeenCalledWith( "Plugin '%s' clientConfig() failed, skipping its config: %O", @@ -457,7 +439,7 @@ describe("ServerPlugin", () => { process.env.NODE_ENV = "production"; vi.mocked(fs.existsSync).mockReturnValue(true); - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); await plugin.start(); @@ -470,7 +452,7 @@ describe("ServerPlugin", () => { process.env.NODE_ENV = "production"; vi.mocked(fs.existsSync).mockReturnValue(false); - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); await plugin.start(); @@ -479,8 +461,8 @@ describe("ServerPlugin", () => { }); describe("extend", () => { - test("should add extension function when autoStart is false", () => { - const plugin = new ServerPlugin({ autoStart: false }); + test("should add extension function and return plugin for chaining", () => { + const plugin = new ServerPlugin({}); const extensionFn = vi.fn(); const result = plugin.extend(extensionFn); @@ -488,17 +470,8 @@ describe("ServerPlugin", () => { expect(result).toBe(plugin); }); - test("should throw when autoStart is true", () => { - const plugin = new ServerPlugin({ autoStart: true }); - const extensionFn = vi.fn(); - - expect(() => plugin.extend(extensionFn)).toThrow( - "Cannot extend server when autoStart is true", - ); - }); - test("should call extension functions during start", async () => { - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); const extensionFn = vi.fn(); plugin.extend(extensionFn); @@ -508,17 +481,18 @@ describe("ServerPlugin", () => { }); }); - describe("getServer", () => { - test("should throw when autoStart is true", () => { - const plugin = new ServerPlugin({ autoStart: true }); + describe("exports().start() trap", () => { + test("should throw migration error when start() is called via exports", () => { + const plugin = new ServerPlugin({}); + const exported = plugin.exports(); - expect(() => plugin.getServer()).toThrow( - "Cannot get server when autoStart is true", - ); + expect(() => exported.start()).toThrow("server.start() has been removed"); }); + }); + describe("getServer", () => { test("should throw when server not started", () => { - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); expect(() => plugin.getServer()).toThrow( "Server not started. Please start the server first by calling the start() method", @@ -526,7 +500,7 @@ describe("ServerPlugin", () => { }); test("should return server after start", async () => { - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); await plugin.start(); const server = plugin.getServer(); @@ -553,7 +527,7 @@ describe("ServerPlugin", () => { describe("logStartupInfo", () => { test("logs remote tunnel controller disabled when missing", () => { mockLoggerDebug.mockClear(); - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); (plugin as any).remoteTunnelController = undefined; (plugin as any).logStartupInfo(); @@ -565,7 +539,7 @@ describe("ServerPlugin", () => { test("logs remote tunnel allowed/active when controller present", () => { mockLoggerDebug.mockClear(); - const plugin = new ServerPlugin({ autoStart: false }); + const plugin = new ServerPlugin({}); (plugin as any).remoteTunnelController = { isAllowedByEnv: () => true, isActive: () => true, @@ -607,7 +581,6 @@ describe("ServerPlugin", () => { .mockImplementation(((_code?: number) => undefined) as any); const plugin = new ServerPlugin({ - autoStart: false, plugins: { ok: { name: "ok", diff --git a/packages/appkit/src/plugins/server/types.ts b/packages/appkit/src/plugins/server/types.ts index e187cacc..f9f6ebce 100644 --- a/packages/appkit/src/plugins/server/types.ts +++ b/packages/appkit/src/plugins/server/types.ts @@ -5,6 +5,5 @@ export interface ServerConfig extends BasePluginConfig { port?: number; plugins?: Record; staticPath?: string; - autoStart?: boolean; host?: string; } diff --git a/packages/shared/src/cli/commands/codemod/customize-callback.ts b/packages/shared/src/cli/commands/codemod/customize-callback.ts new file mode 100644 index 00000000..9d4ac40f --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/customize-callback.ts @@ -0,0 +1,478 @@ +import fs from "node:fs"; +import path from "node:path"; +import { Lang, parse } from "@ast-grep/napi"; +import { Command } from "commander"; + +const SEARCH_DIRS = ["server", "src", "."]; +const CANDIDATE_NAMES = ["server.ts", "index.ts"]; +const SKIP_DIRS = new Set(["node_modules", "dist", "build", ".git"]); + +function findServerEntryFiles(rootDir: string): string[] { + const results: string[] = []; + + for (const dir of SEARCH_DIRS) { + const absDir = path.resolve(rootDir, dir); + if (!fs.existsSync(absDir)) continue; + + const files = + dir === "." + ? CANDIDATE_NAMES.map((n) => path.join(absDir, n)).filter(fs.existsSync) + : findTsFiles(absDir); + + for (const file of files) { + const content = fs.readFileSync(file, "utf-8"); + if ( + content.includes("createApp") && + content.includes("@databricks/appkit") + ) { + results.push(file); + } + } + } + + return [...new Set(results)]; +} + +function findTsFiles(dir: string, files: string[] = []): string[] { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return files; + } + + for (const entry of entries) { + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (SKIP_DIRS.has(entry.name)) continue; + findTsFiles(fullPath, files); + } else if (entry.isFile() && entry.name.endsWith(".ts")) { + files.push(fullPath); + } + } + + return files; +} + +function isAlreadyMigrated(content: string): boolean { + const ast = parse(Lang.TypeScript, content); + const root = ast.root(); + return root.findAll("createApp({ $$$PROPS })").some((match) => { + const text = match.text(); + return /\bonPluginsReady\s*[(:]/.test(text); + }); +} + +/** + * Find the index of the matching closing delimiter for an opening one. + * Supports (), {}, and []. + */ +function findMatchingClose(content: string, openIdx: number): number { + const open = content[openIdx]; + const closeMap: Record = { + "(": ")", + "{": "}", + "[": "]", + }; + const close = closeMap[open]; + if (!close) return -1; + + let depth = 1; + let i = openIdx + 1; + while (i < content.length && depth > 0) { + const ch = content[i]; + if (ch === open) depth++; + else if (ch === close) depth--; + + // skip string literals + if (ch === '"' || ch === "'" || ch === "`") { + i = skipString(content, i); + continue; + } + i++; + } + return depth === 0 ? i - 1 : -1; +} + +function skipString(content: string, startIdx: number): number { + const quote = content[startIdx]; + let i = startIdx + 1; + while (i < content.length) { + if (content[i] === "\\") { + i += 2; + continue; + } + if (content[i] === quote) return i + 1; + i++; + } + return i; +} + +function stripAutoStartFromServerCalls(content: string): string { + return content.replace( + /server\(\{([^}]*)\}\)/g, + (_fullMatch, propsStr: string) => { + const cleaned = propsStr + .replace(/autoStart\s*:\s*(true|false)\s*,?\s*/g, "") + .replace(/,\s*$/, "") + .trim(); + if (!cleaned) return "server()"; + return `server({ ${cleaned} })`; + }, + ); +} + +interface MigrationResult { + migrated: boolean; + content: string; + warnings: string[]; +} + +function migratePatternA(content: string): MigrationResult { + const warnings: string[] = []; + + // Find createApp(...).then( + const createAppIdx = content.indexOf("createApp("); + if (createAppIdx === -1) return { migrated: false, content, warnings }; + + // Find the opening paren of createApp( + const configOpenParen = content.indexOf("(", createAppIdx); + const configCloseParen = findMatchingClose(content, configOpenParen); + if (configCloseParen === -1) return { migrated: false, content, warnings }; + + // Check for .then( after the closing paren + const afterCreateApp = content.slice(configCloseParen + 1); + const thenMatch = afterCreateApp.match(/^\s*\.then\s*\(/); + if (!thenMatch) return { migrated: false, content, warnings }; + + const thenStart = configCloseParen + 1 + afterCreateApp.indexOf(".then"); + const thenOpenParen = content.indexOf("(", thenStart + 4); + const thenCloseParen = findMatchingClose(content, thenOpenParen); + if (thenCloseParen === -1) return { migrated: false, content, warnings }; + + // Extract the callback inside .then(...) + const thenRaw = content.slice(thenOpenParen + 1, thenCloseParen); + const thenInner = thenRaw.trim(); + + // Parse callback: (param) => { body } or async (param) => { body } + const callbackMatch = thenInner.match( + /^(?:async\s+)?\(\s*(\w+)\s*\)\s*=>\s*\{/, + ); + if (!callbackMatch) return { migrated: false, content, warnings }; + + const paramName = callbackMatch[1]; + const bodyOpenBrace = thenOpenParen + 1 + thenRaw.indexOf("{"); + const bodyCloseBrace = findMatchingClose(content, bodyOpenBrace); + if (bodyCloseBrace === -1) return { migrated: false, content, warnings }; + + let callbackBody = content.slice(bodyOpenBrace + 1, bodyCloseBrace).trim(); + + // Remove .start() calls from the body + callbackBody = callbackBody + .replace(/\n\s*\.start\(\s*\)\s*;?/g, ";") + .replace(/\.start\(\s*\)/g, "") + .trim(); + + // Clean up trailing semicolons + if (callbackBody.endsWith(";")) { + // fine + } else if (!callbackBody.endsWith("}")) { + callbackBody += ";"; + } + + // Check for .catch() after .then(...) using brace-aware parsing + const afterThenClose = content.slice(thenCloseParen + 1); + const catchPatternMatch = afterThenClose.match(/^\s*(?:\)\s*)?\.catch\s*\(/); + + let catchSuffix: string; + let consumeAfterThen: number; + + if (catchPatternMatch) { + const catchOpenParen = thenCloseParen + 1 + catchPatternMatch[0].length - 1; + const catchCloseParen = findMatchingClose(content, catchOpenParen); + if (catchCloseParen !== -1) { + const catchArg = content + .slice(catchOpenParen + 1, catchCloseParen) + .trim(); + catchSuffix = `.catch(${catchArg})`; + consumeAfterThen = catchCloseParen + 1 - (thenCloseParen + 1); + } else { + catchSuffix = ".catch(console.error)"; + consumeAfterThen = 0; + } + } else { + catchSuffix = ".catch(console.error)"; + consumeAfterThen = 0; + } + + // Build the onPluginsReady property + const configStr = content.slice(configOpenParen + 1, configCloseParen); + const lastBraceIdx = configStr.lastIndexOf("}"); + if (lastBraceIdx === -1) return { migrated: false, content, warnings }; + + const beforeLastBrace = configStr.slice(0, lastBraceIdx).trimEnd(); + const needsComma = beforeLastBrace.endsWith(",") ? "" : ","; + + // Indent the body properly + const bodyLines = callbackBody.split("\n"); + const indentedBody = bodyLines + .map((line) => ` ${line.trimStart()}`) + .join("\n"); + + const onPluginsReadyProp = `${needsComma}\n onPluginsReady(${paramName}) {\n${indentedBody}\n },`; + const newConfig = `${beforeLastBrace}${onPluginsReadyProp}\n}`; + + // Build the replacement + const endIdx = thenCloseParen + 1 + consumeAfterThen; + // Consume trailing ) ; and whitespace + let finalEnd = endIdx; + const trailing = content.slice(finalEnd).match(/^\s*\)?\s*;?\s*/); + if (trailing) finalEnd += trailing[0].length; + + const newContent = + content.slice(0, createAppIdx) + + `createApp(${newConfig})${catchSuffix};` + + content.slice(finalEnd); + + return { migrated: true, content: newContent, warnings }; +} + +function migratePatternB(content: string): MigrationResult { + const warnings: string[] = []; + + // Match: const/let varName = await createApp({...}); + const awaitPattern = /(?:const|let)\s+(\w+)\s*=\s*await\s+createApp\s*\(/; + + const match = content.match(awaitPattern); + if (!match) return { migrated: false, content, warnings }; + + const varName = match[1]; + const matchIdx = content.indexOf(match[0]); + + // Find the createApp(...) closing paren + const configOpenParen = matchIdx + match[0].length - 1; + const configCloseParen = findMatchingClose(content, configOpenParen); + if (configCloseParen === -1) return { migrated: false, content, warnings }; + + // Find the semicolon after the createApp call + const afterCall = content.slice(configCloseParen + 1); + const semiMatch = afterCall.match(/^\s*;/); + const createAppEnd = + configCloseParen + 1 + (semiMatch ? semiMatch[0].length : 0); + + // Find all uses of varName after the createApp call + const afterCreateApp = content.slice(createAppEnd); + const varUsagePattern = new RegExp(`\\b${varName}\\.(\\w+)`, "g"); + + const usages: { plugin: string; index: number }[] = []; + for (const usageMatch of afterCreateApp.matchAll(varUsagePattern)) { + usages.push({ plugin: usageMatch[1], index: usageMatch.index }); + } + + // Check for non-server usage + const nonServerUsage = usages.filter((u) => u.plugin !== "server"); + if (nonServerUsage.length > 0) { + warnings.push( + `Found additional usage of '${varName}' handle outside server.extend/start. Please migrate manually.`, + ); + return { migrated: false, content, warnings }; + } + + // Find the extend call(s) and start call in the after-createApp region + const extendPattern = new RegExp( + `\\b${varName}\\.server\\.extend\\s*\\(`, + "g", + ); + const startPattern = new RegExp( + `(?:await\\s+)?${varName}\\.server\\.start\\s*\\(\\s*\\)\\s*;`, + ); + + const extendMatches = [...afterCreateApp.matchAll(extendPattern)]; + if (extendMatches.length > 1) { + warnings.push( + `Found ${extendMatches.length} server.extend() calls. Please migrate manually.`, + ); + return { migrated: false, content, warnings }; + } + + const extendExec = extendMatches[0] ?? null; + const startExec = startPattern.exec(afterCreateApp); + + if (!startExec) return { migrated: false, content, warnings }; + + // Extract the extend call's argument + let extendArg = ""; + let extendFullStatement = ""; + if (extendExec) { + const extendOpenParen = + createAppEnd + extendExec.index + extendExec[0].length - 1; + const extendCloseParen = findMatchingClose(content, extendOpenParen); + if (extendCloseParen !== -1) { + extendArg = content.slice(extendOpenParen + 1, extendCloseParen).trim(); + // Find the full statement including trailing semicolon + const stmtStart = createAppEnd + extendExec.index; + let stmtEnd = extendCloseParen + 1; + const afterExtend = content.slice(stmtEnd); + const trailingSemi = afterExtend.match(/^\s*;/); + if (trailingSemi) stmtEnd += trailingSemi[0].length; + extendFullStatement = content.slice(stmtStart, stmtEnd); + } + } + + const startFullStatement = startExec[0]; + + // Build the onPluginsReady callback + const configStr = content.slice(configOpenParen + 1, configCloseParen); + const lastBraceIdx = configStr.lastIndexOf("}"); + if (lastBraceIdx === -1) return { migrated: false, content, warnings }; + + const beforeLastBrace = configStr.slice(0, lastBraceIdx).trimEnd(); + const needsComma = beforeLastBrace.endsWith(",") ? "" : ","; + + let onPluginsReadyProp: string; + if (extendArg) { + onPluginsReadyProp = + `${needsComma}\n onPluginsReady(${varName}) {\n` + + ` ${varName}.server.extend(${extendArg});\n` + + " },"; + } else { + onPluginsReadyProp = ""; + } + + const newConfig = `${beforeLastBrace}${onPluginsReadyProp}\n}`; + const newCreateApp = `await createApp(${newConfig});`; + + // Replace: remove const declaration, replace with plain await, remove extend + start + let result = content.slice(0, matchIdx) + newCreateApp; + let remaining = afterCreateApp; + + if (extendFullStatement) { + remaining = remaining.replace(extendFullStatement, ""); + } + remaining = remaining.replace(startFullStatement, ""); + + // Clean up consecutive blank lines + remaining = remaining.replace(/\n\s*\n\s*\n/g, "\n\n"); + + result += remaining; + + return { migrated: true, content: result, warnings }; +} + +export function migrateFile(filePath: string): MigrationResult { + const original = fs.readFileSync(filePath, "utf-8"); + + if (isAlreadyMigrated(original)) { + return { + migrated: false, + content: original, + warnings: ["Already migrated -- no changes needed."], + }; + } + + const content = stripAutoStartFromServerCalls(original); + const allWarnings: string[] = []; + + // Try Pattern A first + const patternA = migratePatternA(content); + if (patternA.migrated) { + allWarnings.push(...patternA.warnings); + return { + migrated: true, + content: patternA.content, + warnings: allWarnings, + }; + } + allWarnings.push(...patternA.warnings); + + // Try Pattern B + const patternB = migratePatternB(content); + if (patternB.migrated) { + allWarnings.push(...patternB.warnings); + return { + migrated: true, + content: patternB.content, + warnings: allWarnings, + }; + } + allWarnings.push(...patternB.warnings); + + // Check if autoStart was stripped (content changed but no pattern matched) + if (content !== original) { + return { migrated: true, content, warnings: allWarnings }; + } + + return { migrated: false, content: original, warnings: allWarnings }; +} + +function runCodemod(options: { path?: string; write?: boolean }) { + const rootDir = process.cwd(); + const write = options.write ?? false; + + let files: string[]; + if (options.path) { + const absPath = path.resolve(rootDir, options.path); + if (!fs.existsSync(absPath)) { + console.error(`File not found: ${absPath}`); + process.exit(1); + } + files = [absPath]; + } else { + files = findServerEntryFiles(rootDir); + } + + if (files.length === 0) { + console.log("No files found importing createApp from @databricks/appkit."); + console.log("Use --path to specify a file explicitly."); + process.exit(0); + } + + let hasChanges = false; + + for (const file of files) { + const relPath = path.relative(rootDir, file); + const result = migrateFile(file); + + for (const warning of result.warnings) { + console.log(` ${relPath}: ${warning}`); + } + + if (!result.migrated) { + if (result.warnings.length === 0) { + console.log(` ${relPath}: No migration needed.`); + } + continue; + } + + hasChanges = true; + + if (write) { + fs.writeFileSync(file, result.content, "utf-8"); + console.log(` ${relPath}: Migrated successfully.`); + } else { + console.log(`\n--- ${relPath} (dry run) ---`); + console.log(result.content); + console.log("---"); + } + } + + if (hasChanges && !write) { + console.log("\nDry run complete. Run with --write to apply changes."); + } +} + +export const customizeCallbackCommand = new Command("customize-callback") + .description( + "Migrate createApp usage from autoStart/extend/start pattern to onPluginsReady callback (formerly customize)", + ) + .option("--path ", "Path to the server entry file to migrate") + .option("--write", "Apply changes (default: dry-run)", false) + .addHelpText( + "after", + ` +Examples: + $ appkit codemod customize-callback # dry-run, auto-detect files + $ appkit codemod customize-callback --write # apply changes + $ appkit codemod customize-callback --path server.ts # migrate a specific file`, + ) + .action(runCodemod); diff --git a/packages/shared/src/cli/commands/codemod/index.ts b/packages/shared/src/cli/commands/codemod/index.ts new file mode 100644 index 00000000..0574427a --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/index.ts @@ -0,0 +1,17 @@ +import { Command } from "commander"; +import { customizeCallbackCommand } from "./customize-callback"; + +/** + * Parent command for codemod operations. + * Subcommands: + * - customize-callback: Migrate from autoStart/extend/start to onPluginsReady callback + */ +export const codemodCommand = new Command("codemod") + .description("Run codemods to migrate to newer AppKit APIs") + .addCommand(customizeCallbackCommand) + .addHelpText( + "after", + ` +Examples: + $ appkit codemod customize-callback --write`, + ); diff --git a/packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts b/packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts new file mode 100644 index 00000000..fc2c6e92 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts @@ -0,0 +1,129 @@ +import fs from "node:fs"; +import path from "node:path"; +import { describe, expect, test } from "vitest"; +import { migrateFile } from "../customize-callback"; + +const fixturesDir = path.join(__dirname, "fixtures"); + +function readFixture(name: string): string { + return fs.readFileSync(path.join(fixturesDir, name), "utf-8"); +} + +describe("onPluginsReady-callback codemod", () => { + describe("Pattern A: .then() chain", () => { + test("migrates .then chain without .catch, adds .catch(console.error)", () => { + const fixturePath = path.join(fixturesDir, "pattern-a.input.ts"); + const result = migrateFile(fixturePath); + + expect(result.migrated).toBe(true); + expect(result.content).toContain("onPluginsReady(appkit)"); + expect(result.content).not.toContain(".then("); + expect(result.content).not.toContain(".start()"); + expect(result.content).not.toContain("autoStart"); + expect(result.content).toContain(".catch(console.error)"); + expect(result.content).toContain("server()"); + }); + + test("migrates .then chain with existing .catch, preserves it", () => { + const fixturePath = path.join( + fixturesDir, + "pattern-a-with-catch.input.ts", + ); + const result = migrateFile(fixturePath); + + expect(result.migrated).toBe(true); + expect(result.content).toContain("onPluginsReady(appkit)"); + expect(result.content).not.toContain(".then("); + expect(result.content).not.toContain(".start()"); + expect(result.content).toContain(".catch(console.error)"); + expect(result.content).toContain("server()"); + }); + + test("preserves the extend callback content", () => { + const fixturePath = path.join(fixturesDir, "pattern-a.input.ts"); + const result = migrateFile(fixturePath); + + expect(result.content).toContain('app.get("/custom"'); + expect(result.content).toContain("res.json({ ok: true })"); + }); + + test("preserves arrow function .catch handler with parens", () => { + const fixturePath = path.join( + fixturesDir, + "pattern-a-arrow-catch.input.ts", + ); + const result = migrateFile(fixturePath); + + expect(result.migrated).toBe(true); + expect(result.content).toContain(".catch((err) => console.error(err))"); + expect(result.content).not.toContain(".then("); + expect(result.content).not.toContain(".start()"); + }); + }); + + describe("Pattern B: await + imperative", () => { + test("migrates await pattern with extend + start", () => { + const fixturePath = path.join(fixturesDir, "pattern-b.input.ts"); + const result = migrateFile(fixturePath); + + expect(result.migrated).toBe(true); + expect(result.content).toContain("onPluginsReady(appkit)"); + expect(result.content).not.toContain("appkit.server.start()"); + expect(result.content).not.toContain("autoStart"); + expect(result.content).toContain("server()"); + }); + + test("bails out when non-server usage of appkit handle exists", () => { + const fixturePath = path.join(fixturesDir, "pattern-b-complex.input.ts"); + const result = migrateFile(fixturePath); + + expect(result.warnings.some((w) => w.includes("migrate manually"))).toBe( + true, + ); + expect(result.content).toContain("server()"); + expect(result.content).not.toContain("autoStart"); + }); + + test("bails out when multiple .extend() calls exist", () => { + const fixturePath = path.join( + fixturesDir, + "pattern-b-multi-extend.input.ts", + ); + const result = migrateFile(fixturePath); + + expect(result.warnings.some((w) => w.includes("migrate manually"))).toBe( + true, + ); + expect(result.content).toContain("server()"); + expect(result.content).not.toContain("autoStart"); + }); + }); + + describe("autoStart stripping", () => { + test("strips autoStart: true and preserves other config", () => { + const fixturePath = path.join( + fixturesDir, + "autostart-true-with-port.input.ts", + ); + const result = migrateFile(fixturePath); + + expect(result.migrated).toBe(true); + expect(result.content).not.toContain("autoStart"); + expect(result.content).toContain("port: 3000"); + expect(result.content).toContain("server({"); + }); + }); + + describe("idempotency", () => { + test("no-ops on already migrated file", () => { + const fixturePath = path.join(fixturesDir, "already-migrated.input.ts"); + const result = migrateFile(fixturePath); + + expect(result.migrated).toBe(false); + expect(result.warnings.some((w) => w.includes("Already migrated"))).toBe( + true, + ); + expect(result.content).toBe(readFixture("already-migrated.input.ts")); + }); + }); +}); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts new file mode 100644 index 00000000..ab1cf6d0 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts @@ -0,0 +1,10 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +createApp({ + plugins: [server(), analytics({})], + onPluginsReady(appkit) { + appkit.server.extend((app) => { + app.get("/custom", (_req, res) => res.json({ ok: true })); + }); + }, +}).catch(console.error); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts new file mode 100644 index 00000000..96b70a4f --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts @@ -0,0 +1,5 @@ +import { createApp, server } from "@databricks/appkit"; + +createApp({ + plugins: [server({ autoStart: true, port: 3000 })], +}).catch(console.error); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-arrow-catch.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-arrow-catch.input.ts new file mode 100644 index 00000000..b6ae8c8a --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-arrow-catch.input.ts @@ -0,0 +1,15 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +createApp({ + plugins: [server({ autoStart: false }), analytics({})], +}) + .then((appkit) => { + appkit.server + .extend((app) => { + app.get("/custom", (_req, res) => { + res.json({ ok: true }); + }); + }) + .start(); + }) + .catch((err) => console.error(err)); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts new file mode 100644 index 00000000..faa04d5e --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts @@ -0,0 +1,15 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +createApp({ + plugins: [server({ autoStart: false }), analytics({})], +}) + .then((appkit) => { + appkit.server + .extend((app) => { + app.get("/custom", (_req, res) => { + res.json({ ok: true }); + }); + }) + .start(); + }) + .catch(console.error); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts new file mode 100644 index 00000000..73523d6a --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts @@ -0,0 +1,13 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +createApp({ + plugins: [server({ autoStart: false }), analytics({})], +}).then((appkit) => { + appkit.server + .extend((app) => { + app.get("/custom", (_req, res) => { + res.json({ ok: true }); + }); + }) + .start(); +}); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts new file mode 100644 index 00000000..c1fb25fa --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts @@ -0,0 +1,15 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +const appkit = await createApp({ + plugins: [server({ autoStart: false }), analytics({})], +}); + +appkit.server.extend((app) => { + app.get("/custom", (_req, res) => { + res.json({ ok: true }); + }); +}); + +appkit.analytics.query("SELECT 1"); + +await appkit.server.start(); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-multi-extend.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-multi-extend.input.ts new file mode 100644 index 00000000..dded09f2 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-multi-extend.input.ts @@ -0,0 +1,15 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +const appkit = await createApp({ + plugins: [server({ autoStart: false }), analytics({})], +}); + +appkit.server.extend((app) => { + app.get("/one", (_req, res) => res.json({ route: 1 })); +}); + +appkit.server.extend((app) => { + app.get("/two", (_req, res) => res.json({ route: 2 })); +}); + +await appkit.server.start(); diff --git a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts new file mode 100644 index 00000000..b56c0048 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts @@ -0,0 +1,13 @@ +import { analytics, createApp, server } from "@databricks/appkit"; + +const appkit = await createApp({ + plugins: [server({ autoStart: false }), analytics({})], +}); + +appkit.server.extend((app) => { + app.get("/custom", (_req, res) => { + res.json({ ok: true }); + }); +}); + +await appkit.server.start(); diff --git a/packages/shared/src/cli/index.ts b/packages/shared/src/cli/index.ts index 71f09e6f..4d0ed65b 100644 --- a/packages/shared/src/cli/index.ts +++ b/packages/shared/src/cli/index.ts @@ -4,6 +4,7 @@ import { readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { Command } from "commander"; +import { codemodCommand } from "./commands/codemod/index.js"; import { docsCommand } from "./commands/docs.js"; import { generateTypesCommand } from "./commands/generate-types.js"; import { lintCommand } from "./commands/lint.js"; @@ -26,5 +27,6 @@ cmd.addCommand(generateTypesCommand); cmd.addCommand(lintCommand); cmd.addCommand(docsCommand); cmd.addCommand(pluginCommand); +cmd.addCommand(codemodCommand); cmd.parse(); diff --git a/packages/shared/tsconfig.json b/packages/shared/tsconfig.json index 4a6e68b3..5e195c3b 100644 --- a/packages/shared/tsconfig.json +++ b/packages/shared/tsconfig.json @@ -8,5 +8,5 @@ } }, "include": ["src/**/*"], - "exclude": ["node_modules", "dist"] + "exclude": ["node_modules", "dist", "src/**/fixtures"] } diff --git a/template/server/server.ts b/template/server/server.ts index 214ac1ce..e28f3ef4 100644 --- a/template/server/server.ts +++ b/template/server/server.ts @@ -5,24 +5,13 @@ import { setupSampleLakebaseRoutes } from './routes/lakebase/todo-routes'; createApp({ plugins: [ -{{- if .plugins.lakebase}} - server({ autoStart: false }), -{{- range $name, $_ := .plugins}} -{{- if ne $name "server"}} - {{$name}}(), -{{- end}} -{{- end}} -{{- else}} {{- range $name, $_ := .plugins}} {{$name}}(), -{{- end}} {{- end}} ], -}) {{- if .plugins.lakebase}} - .then(async (appkit) => { + async onPluginsReady(appkit) { await setupSampleLakebaseRoutes(appkit); - await appkit.server.start(); - }) + }, {{- end}} - .catch(console.error); +}).catch(console.error);