From 044252916c45051d4db186dfb94ce768ad54a504 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 16 Apr 2026 12:40:44 +0200 Subject: [PATCH 1/8] feat: add customize callback to createApp, remove autoStart Replace the post-await extend/start ceremony with a declarative `customize` callback on createApp config. The callback runs after plugin setup but before the server starts, giving access to the full appkit handle for registering custom routes or async setup. - Add `customize` option to createApp config - Server start is now orchestrated by createApp (lookup by name) - Remove `autoStart` from public API, ServerConfig, and manifest - Remove `start()` from server plugin exports - Remove autoStart guards from extend() and getServer() - Remove ServerError.autoStartConflict() - Migrate dev-playground, template, and all tests Signed-off-by: MarioCadenas --- apps/dev-playground/server/index.ts | 13 +-- .../shared/appkit-types/analytics.d.ts | 8 +- docs/docs/api/appkit/Class.ServerError.md | 21 ---- docs/docs/api/appkit/Function.createApp.md | 21 ++-- docs/docs/plugins/server.md | 33 ++++-- packages/appkit/src/core/appkit.ts | 32 ++++-- packages/appkit/src/errors/server.ts | 10 -- .../appkit/src/errors/tests/errors.test.ts | 6 - .../tests/analytics.integration.test.ts | 2 - .../files/tests/plugin.integration.test.ts | 2 - packages/appkit/src/plugins/server/index.ts | 40 +++---- .../appkit/src/plugins/server/manifest.json | 5 - .../server/tests/server.integration.test.ts | 105 +++++++++++++++--- .../src/plugins/server/tests/server.test.ts | 80 ++++--------- packages/appkit/src/plugins/server/types.ts | 1 - template/server/server.ts | 17 +-- 16 files changed, 196 insertions(+), 200 deletions(-) diff --git a/apps/dev-playground/server/index.ts b/apps/dev-playground/server/index.ts index 8a77b76c1..70ba857f2 100644 --- a/apps/dev-playground/server/index.ts +++ b/apps/dev-playground/server/index.ts @@ -50,7 +50,7 @@ const adminOnly: FilePolicy = (action, _resource, user) => { createApp({ plugins: [ - server({ autoStart: false }), + server(), reconnect(), telemetryExamples(), analytics({}), @@ -95,9 +95,8 @@ createApp({ // }), ], ...(process.env.APPKIT_E2E_TEST && { client: createMockClient() }), -}).then((appkit) => { - appkit.server - .extend((app) => { + customize(appkit) { + appkit.server.extend((app) => { app.get("/sp", (_req, res) => { appkit.analytics .query("SELECT * FROM samples.nyctaxi.trips;") @@ -195,9 +194,9 @@ createApp({ results, }); }); - }) - .start(); -}); + }); + }, +}).catch(console.error); type ProbeResult = { volume: string; diff --git a/apps/dev-playground/shared/appkit-types/analytics.d.ts b/apps/dev-playground/shared/appkit-types/analytics.d.ts index 0e0ae0b07..43666dd06 100644 --- a/apps/dev-playground/shared/appkit-types/analytics.d.ts +++ b/apps/dev-playground/shared/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/docs/docs/api/appkit/Class.ServerError.md b/docs/docs/api/appkit/Class.ServerError.md index d3dce68ee..cce86cad0 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 cb7033868..552ceaca2 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; + customize?: (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 `customize` 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`; `customize?`: (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\>; `plugins?`: `T`; `telemetry?`: [`TelemetryConfig`](Interface.TelemetryConfig.md); \} | | `config.cache?` | [`CacheConfig`](Interface.CacheConfig.md) | | `config.client?` | `WorkspaceClient` | +| `config.customize?` | (`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({})], + customize(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 389828dca..a97f4361d 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 `customize` 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()], + customize(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 `customize` callback also supports async operations: -await appkit.server.start(); +```ts +await createApp({ + plugins: [server()], + async customize(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 a2cba9949..07c735d36 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; + customize?: (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.customize?.(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 `customize` 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 customize * ```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({})], + * customize(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; + customize?: (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 6af5b59f0..d45148d82 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 c404a18f1..347ce1c0d 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 cb73394ad..0cec22984 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 da90760db..3c8ff74ec 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 e7b9b31ad..88df4be47 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 `customize` callback has run. + * * @example * ```ts * createApp({ - * plugins: [server(), telemetryExamples(), analytics({})], + * plugins: [server(), analytics({})], + * customize(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, }; @@ -65,12 +71,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 +80,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 +144,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 +158,13 @@ export class ServerPlugin extends Plugin { /** * Extend the server with custom routes or middleware. * + * Call this inside the `customize` 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 +379,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); diff --git a/packages/appkit/src/plugins/server/manifest.json b/packages/appkit/src/plugins/server/manifest.json index 11822beb7..1112fbf58 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 c3a646eae..ca44168af 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 customize", () => { 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, }), ], + customize(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 customize 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 customize 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 customize(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 customize 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 customizeWasCalled = false; + + beforeAll(async () => { + setupDatabricksEnv(); + ServiceContext.reset(); + serviceContextMock = await mockServiceContext(); + + await createApp({ + plugins: [], + customize() { + customizeWasCalled = true; + }, + }); + }); + + afterAll(async () => { + serviceContextMock?.restore(); + }); + + test("customize callback is still called without server plugin", () => { + expect(customizeWasCalled).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 22f181298..57b7d510b 100644 --- a/packages/appkit/src/plugins/server/tests/server.test.ts +++ b/packages/appkit/src/plugins/server/tests/server.test.ts @@ -197,19 +197,16 @@ 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); }); }); 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 +217,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 +230,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 +243,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 +253,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 +280,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 +324,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 +362,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 +389,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 +420,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 +433,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 +446,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 +455,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 +464,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); @@ -509,16 +476,8 @@ describe("ServerPlugin", () => { }); describe("getServer", () => { - test("should throw when autoStart is true", () => { - const plugin = new ServerPlugin({ autoStart: true }); - - expect(() => plugin.getServer()).toThrow( - "Cannot get server when autoStart is true", - ); - }); - 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 +485,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 +512,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 +524,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 +566,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 e187caccf..f9f6ebce3 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/template/server/server.ts b/template/server/server.ts index 214ac1cea..c084ef7b9 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 customize(appkit) { await setupSampleLakebaseRoutes(appkit); - await appkit.server.start(); - }) + }, {{- end}} - .catch(console.error); +}).catch(console.error); From b8b17a50d660b990e97100bb40ccad079ac23490 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 20 Apr 2026 12:18:02 +0200 Subject: [PATCH 2/8] feat: rename customize to onPluginsReady, add codemod CLI and runtime detection Rename the lifecycle hook from `customize` to `onPluginsReady` to clearly communicate when it fires (after plugins are ready, before server starts). Add `appkit codemod customize-callback` CLI command that auto-migrates old autoStart/extend/start patterns to the new onPluginsReady callback. Supports both .then() chain (Pattern A) and await + imperative (Pattern B, with bail-out for complex cases). Add runtime detection that throws helpful errors when users pass autoStart to server() or call server.start() after upgrading, directing them to run the codemod. Signed-off-by: MarioCadenas --- apps/dev-playground/server/index.ts | 2 +- docs/docs/api/appkit/Function.createApp.md | 10 +- docs/docs/plugins/server.md | 8 +- packages/appkit/src/core/appkit.ts | 12 +- packages/appkit/src/plugins/server/index.ts | 26 +- .../server/tests/server.integration.test.ts | 22 +- .../src/plugins/server/tests/server.test.ts | 15 + .../commands/codemod/customize-callback.ts | 460 ++++++++++++++++++ .../shared/src/cli/commands/codemod/index.ts | 17 + .../codemod/tests/customize-callback.test.ts | 103 ++++ .../tests/fixtures/already-migrated.input.ts | 10 + .../autostart-true-with-port.input.ts | 5 + .../fixtures/pattern-a-with-catch.input.ts | 15 + .../codemod/tests/fixtures/pattern-a.input.ts | 13 + .../tests/fixtures/pattern-b-complex.input.ts | 15 + .../codemod/tests/fixtures/pattern-b.input.ts | 13 + packages/shared/src/cli/index.ts | 2 + template/server/server.ts | 2 +- 18 files changed, 719 insertions(+), 31 deletions(-) create mode 100644 packages/shared/src/cli/commands/codemod/customize-callback.ts create mode 100644 packages/shared/src/cli/commands/codemod/index.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts diff --git a/apps/dev-playground/server/index.ts b/apps/dev-playground/server/index.ts index 70ba857f2..94f1cc128 100644 --- a/apps/dev-playground/server/index.ts +++ b/apps/dev-playground/server/index.ts @@ -95,7 +95,7 @@ createApp({ // }), ], ...(process.env.APPKIT_E2E_TEST && { client: createMockClient() }), - customize(appkit) { + onPluginsReady(appkit) { appkit.server.extend((app) => { app.get("/sp", (_req, res) => { appkit.analytics diff --git a/docs/docs/api/appkit/Function.createApp.md b/docs/docs/api/appkit/Function.createApp.md index 552ceaca2..6a0b7cb2a 100644 --- a/docs/docs/api/appkit/Function.createApp.md +++ b/docs/docs/api/appkit/Function.createApp.md @@ -4,7 +4,7 @@ function createApp(config: { cache?: CacheConfig; client?: WorkspaceClient; - customize?: (appkit: PluginMap) => void | Promise; + onPluginsReady?: (appkit: PluginMap) => void | Promise; plugins?: T; telemetry?: TelemetryConfig; }): Promise>; @@ -14,7 +14,7 @@ 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 `customize` callback is provided it runs after plugin setup but +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, @@ -30,10 +30,10 @@ with an `asUser(req)` method for user-scoped execution. | Parameter | Type | | ------ | ------ | -| `config` | \{ `cache?`: [`CacheConfig`](Interface.CacheConfig.md); `client?`: `WorkspaceClient`; `customize?`: (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\>; `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.customize?` | (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\> | +| `config.onPluginsReady?` | (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\> | | `config.plugins?` | `T` | | `config.telemetry?` | [`TelemetryConfig`](Interface.TelemetryConfig.md) | @@ -58,7 +58,7 @@ import { createApp, server, analytics } from "@databricks/appkit"; await createApp({ plugins: [server(), analytics({})], - customize(appkit) { + onPluginsReady(appkit) { appkit.server.extend((app) => { app.get("/custom", (_req, res) => res.json({ ok: true })); }); diff --git a/docs/docs/plugins/server.md b/docs/docs/plugins/server.md index a97f4361d..6cfaa7b73 100644 --- a/docs/docs/plugins/server.md +++ b/docs/docs/plugins/server.md @@ -38,14 +38,14 @@ await createApp({ ## Custom routes example -Use the `customize` callback to extend Express with custom routes before the server starts: +Use the `onPluginsReady` callback to extend Express with custom routes before the server starts: ```ts import { createApp, server } from "@databricks/appkit"; await createApp({ plugins: [server()], - customize(appkit) { + onPluginsReady(appkit) { appkit.server.extend((app) => { app.get("/custom", (_req, res) => res.json({ ok: true })); }); @@ -53,12 +53,12 @@ await createApp({ }); ``` -The `customize` callback also supports async operations: +The `onPluginsReady` callback also supports async operations: ```ts await createApp({ plugins: [server()], - async customize(appkit) { + async onPluginsReady(appkit) { const pool = await initializeDatabase(); appkit.server.extend((app) => { app.get("/data", async (_req, res) => { diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index 07c735d36..40fd1fcd8 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -167,7 +167,7 @@ export class AppKit { telemetry?: TelemetryConfig; cache?: CacheConfig; client?: WorkspaceClient; - customize?: (appkit: PluginMap) => void | Promise; + onPluginsReady?: (appkit: PluginMap) => void | Promise; } = {}, ): Promise> { // Initialize core services @@ -203,7 +203,7 @@ export class AppKit { const handle = instance as unknown as PluginMap; - await config.customize?.(handle); + await config.onPluginsReady?.(handle); const serverPlugin = instance.#pluginInstances.server; if (serverPlugin && typeof (serverPlugin as any).start === "function") { @@ -232,7 +232,7 @@ export class AppKit { * * Initializes telemetry, cache, and service context, then registers plugins * in phase order (core, normal, deferred) and awaits their setup. - * If a `customize` callback is provided it runs after plugin setup but + * 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, @@ -249,13 +249,13 @@ export class AppKit { * }); * ``` * - * @example Server with custom routes via customize + * @example Server with custom routes via onPluginsReady * ```ts * import { createApp, server, analytics } from "@databricks/appkit"; * * await createApp({ * plugins: [server(), analytics({})], - * customize(appkit) { + * onPluginsReady(appkit) { * appkit.server.extend((app) => { * app.get("/custom", (_req, res) => res.json({ ok: true })); * }); @@ -271,7 +271,7 @@ export async function createApp< telemetry?: TelemetryConfig; cache?: CacheConfig; client?: WorkspaceClient; - customize?: (appkit: PluginMap) => void | Promise; + onPluginsReady?: (appkit: PluginMap) => void | Promise; } = {}, ): Promise> { return AppKit._createApp(config); diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index 88df4be47..6c4841148 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -28,13 +28,13 @@ const logger = createLogger("server"); * 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 `customize` callback has run. + * and the optional `onPluginsReady` callback has run. * * @example * ```ts * createApp({ * plugins: [server(), analytics({})], - * customize(appkit) { + * onPluginsReady(appkit) { * appkit.server.extend((app) => { * app.get("/custom", (_req, res) => res.json({ ok: true })); * }); @@ -60,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(); @@ -158,7 +165,7 @@ export class ServerPlugin extends Plugin { /** * Extend the server with custom routes or middleware. * - * Call this inside the `customize` callback of `createApp` to register + * 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. @@ -388,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/tests/server.integration.test.ts b/packages/appkit/src/plugins/server/tests/server.integration.test.ts index ca44168af..0b67e4c06 100644 --- a/packages/appkit/src/plugins/server/tests/server.integration.test.ts +++ b/packages/appkit/src/plugins/server/tests/server.integration.test.ts @@ -167,7 +167,7 @@ describe("ServerPlugin with custom plugin", () => { }); }); -describe("ServerPlugin with extend() via customize", () => { +describe("ServerPlugin with extend() via onPluginsReady", () => { let server: Server; let baseUrl: string; let serviceContextMock: Awaited>; @@ -185,7 +185,7 @@ describe("ServerPlugin with extend() via customize", () => { host: "127.0.0.1", }), ], - customize(appkit) { + onPluginsReady(appkit) { appkit.server.extend((expressApp) => { expressApp.get("/custom", (_req, res) => { res.json({ custom: true }); @@ -212,7 +212,7 @@ describe("ServerPlugin with extend() via customize", () => { } }); - test("custom route via extend() in customize callback works", async () => { + test("custom route via extend() in onPluginsReady callback works", async () => { const response = await fetch(`${baseUrl}/custom`); expect(response.status).toBe(200); @@ -222,7 +222,7 @@ describe("ServerPlugin with extend() via customize", () => { }); }); -describe("createApp with async customize callback", () => { +describe("createApp with async onPluginsReady callback", () => { let server: Server; let baseUrl: string; let serviceContextMock: Awaited>; @@ -240,7 +240,7 @@ describe("createApp with async customize callback", () => { host: "127.0.0.1", }), ], - async customize(appkit) { + async onPluginsReady(appkit) { await new Promise((resolve) => setTimeout(resolve, 10)); appkit.server.extend((expressApp) => { expressApp.get("/async-custom", (_req, res) => { @@ -268,7 +268,7 @@ describe("createApp with async customize callback", () => { } }); - test("async customize callback runs before server starts", async () => { + test("async onPluginsReady callback runs before server starts", async () => { const response = await fetch(`${baseUrl}/async-custom`); expect(response.status).toBe(200); @@ -280,7 +280,7 @@ describe("createApp with async customize callback", () => { describe("createApp without server plugin", () => { let serviceContextMock: Awaited>; - let customizeWasCalled = false; + let onPluginsReadyWasCalled = false; beforeAll(async () => { setupDatabricksEnv(); @@ -289,8 +289,8 @@ describe("createApp without server plugin", () => { await createApp({ plugins: [], - customize() { - customizeWasCalled = true; + onPluginsReady() { + onPluginsReadyWasCalled = true; }, }); }); @@ -299,7 +299,7 @@ describe("createApp without server plugin", () => { serviceContextMock?.restore(); }); - test("customize callback is still called without server plugin", () => { - expect(customizeWasCalled).toBe(true); + 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 57b7d510b..fae11fb5e 100644 --- a/packages/appkit/src/plugins/server/tests/server.test.ts +++ b/packages/appkit/src/plugins/server/tests/server.test.ts @@ -203,6 +203,12 @@ describe("ServerPlugin", () => { expect(config.port).toBe(3000); expect(config.host).toBe("127.0.0.1"); }); + + test("should throw when autoStart is passed", () => { + expect(() => new ServerPlugin({ autoStart: false } as any)).toThrow( + "server({ autoStart }) has been removed", + ); + }); }); describe("DEFAULT_CONFIG", () => { @@ -475,6 +481,15 @@ describe("ServerPlugin", () => { }); }); + describe("exports().start() trap", () => { + test("should throw migration error when start() is called via exports", () => { + const plugin = new ServerPlugin({}); + const exported = plugin.exports(); + + expect(() => exported.start()).toThrow("server.start() has been removed"); + }); + }); + describe("getServer", () => { test("should throw when server not started", () => { const plugin = new ServerPlugin({}); 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 000000000..f13fb354e --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/customize-callback.ts @@ -0,0 +1,460 @@ +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 thenInner = content.slice(thenOpenParen + 1, thenCloseParen).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 + thenInner.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(...) + const afterThenClose = content.slice(thenCloseParen + 1); + const catchMatch = afterThenClose.match(/^\s*\)\s*\.catch\s*\(([^)]*)\)/); + const directCatchMatch = afterThenClose.match(/^\s*\.catch\s*\(([^)]*)\)/); + + let catchSuffix: string; + let consumeAfterThen: number; + + if (directCatchMatch) { + catchSuffix = `.catch(${directCatchMatch[1]})`; + consumeAfterThen = directCatchMatch[0].length; + } else if (catchMatch) { + catchSuffix = `.catch(${catchMatch[1]})`; + consumeAfterThen = catchMatch[0].length; + } 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 and start call in the after-createApp region + const extendPattern = new RegExp(`\\b${varName}\\.server\\.extend\\s*\\(`); + const startPattern = new RegExp( + `(?:await\\s+)?${varName}\\.server\\.start\\s*\\(\\s*\\)\\s*;`, + ); + + const extendExec = extendPattern.exec(afterCreateApp); + 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 000000000..0574427a0 --- /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 000000000..7302faa90 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts @@ -0,0 +1,103 @@ +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 })"); + }); + }); + + 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); + + // Should still strip autoStart but not do the full pattern B migration + 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 000000000..ab1cf6d06 --- /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 000000000..96b70a4f8 --- /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-with-catch.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts new file mode 100644 index 000000000..faa04d5ef --- /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 000000000..73523d6a1 --- /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 000000000..c1fb25fab --- /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.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts new file mode 100644 index 000000000..b56c00487 --- /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 71f09e6f6..4d0ed65b7 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/template/server/server.ts b/template/server/server.ts index c084ef7b9..e28f3ef43 100644 --- a/template/server/server.ts +++ b/template/server/server.ts @@ -10,7 +10,7 @@ createApp({ {{- end}} ], {{- if .plugins.lakebase}} - async customize(appkit) { + async onPluginsReady(appkit) { await setupSampleLakebaseRoutes(appkit); }, {{- end}} From 6689434ebe8ab5d4283b6948be6ab38a12774962 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 20 Apr 2026 19:15:24 +0200 Subject: [PATCH 3/8] fix: exclude codemod fixture files from typecheck The test fixture .ts files import @databricks/appkit which doesn't exist in the shared package, causing tsc to fail in CI. Exclude the fixtures directory from the shared tsconfig. Signed-off-by: MarioCadenas --- packages/shared/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/tsconfig.json b/packages/shared/tsconfig.json index 4a6e68b3e..5e195c3b6 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"] } From 32c880b320d29fa58f892b52360177bc777e7100 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 20 Apr 2026 19:31:29 +0200 Subject: [PATCH 4/8] refactor: split codemod into separate PR Remove the codemod CLI from this PR to keep the review focused on the core lifecycle change. The codemod will land as a follow-up with bug fixes from review. Runtime detection (constructor autoStart throw + exports().start() trap) stays since it's part of the migration story. Signed-off-by: MarioCadenas --- .../commands/codemod/customize-callback.ts | 460 ------------------ .../shared/src/cli/commands/codemod/index.ts | 17 - .../codemod/tests/customize-callback.test.ts | 103 ---- .../tests/fixtures/already-migrated.input.ts | 10 - .../autostart-true-with-port.input.ts | 5 - .../fixtures/pattern-a-with-catch.input.ts | 15 - .../codemod/tests/fixtures/pattern-a.input.ts | 13 - .../tests/fixtures/pattern-b-complex.input.ts | 15 - .../codemod/tests/fixtures/pattern-b.input.ts | 13 - packages/shared/src/cli/index.ts | 2 - packages/shared/tsconfig.json | 2 +- 11 files changed, 1 insertion(+), 654 deletions(-) delete mode 100644 packages/shared/src/cli/commands/codemod/customize-callback.ts delete mode 100644 packages/shared/src/cli/commands/codemod/index.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts delete mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts diff --git a/packages/shared/src/cli/commands/codemod/customize-callback.ts b/packages/shared/src/cli/commands/codemod/customize-callback.ts deleted file mode 100644 index f13fb354e..000000000 --- a/packages/shared/src/cli/commands/codemod/customize-callback.ts +++ /dev/null @@ -1,460 +0,0 @@ -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 thenInner = content.slice(thenOpenParen + 1, thenCloseParen).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 + thenInner.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(...) - const afterThenClose = content.slice(thenCloseParen + 1); - const catchMatch = afterThenClose.match(/^\s*\)\s*\.catch\s*\(([^)]*)\)/); - const directCatchMatch = afterThenClose.match(/^\s*\.catch\s*\(([^)]*)\)/); - - let catchSuffix: string; - let consumeAfterThen: number; - - if (directCatchMatch) { - catchSuffix = `.catch(${directCatchMatch[1]})`; - consumeAfterThen = directCatchMatch[0].length; - } else if (catchMatch) { - catchSuffix = `.catch(${catchMatch[1]})`; - consumeAfterThen = catchMatch[0].length; - } 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 and start call in the after-createApp region - const extendPattern = new RegExp(`\\b${varName}\\.server\\.extend\\s*\\(`); - const startPattern = new RegExp( - `(?:await\\s+)?${varName}\\.server\\.start\\s*\\(\\s*\\)\\s*;`, - ); - - const extendExec = extendPattern.exec(afterCreateApp); - 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 deleted file mode 100644 index 0574427a0..000000000 --- a/packages/shared/src/cli/commands/codemod/index.ts +++ /dev/null @@ -1,17 +0,0 @@ -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 deleted file mode 100644 index 7302faa90..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/customize-callback.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -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 })"); - }); - }); - - 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); - - // Should still strip autoStart but not do the full pattern B migration - 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 deleted file mode 100644 index ab1cf6d06..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts +++ /dev/null @@ -1,10 +0,0 @@ -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 deleted file mode 100644 index 96b70a4f8..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts +++ /dev/null @@ -1,5 +0,0 @@ -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-with-catch.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts deleted file mode 100644 index faa04d5ef..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts +++ /dev/null @@ -1,15 +0,0 @@ -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 deleted file mode 100644 index 73523d6a1..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts +++ /dev/null @@ -1,13 +0,0 @@ -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 deleted file mode 100644 index c1fb25fab..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts +++ /dev/null @@ -1,15 +0,0 @@ -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.input.ts b/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts deleted file mode 100644 index b56c00487..000000000 --- a/packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts +++ /dev/null @@ -1,13 +0,0 @@ -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 4d0ed65b7..71f09e6f6 100644 --- a/packages/shared/src/cli/index.ts +++ b/packages/shared/src/cli/index.ts @@ -4,7 +4,6 @@ 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"; @@ -27,6 +26,5 @@ 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 5e195c3b6..4a6e68b3e 100644 --- a/packages/shared/tsconfig.json +++ b/packages/shared/tsconfig.json @@ -8,5 +8,5 @@ } }, "include": ["src/**/*"], - "exclude": ["node_modules", "dist", "src/**/fixtures"] + "exclude": ["node_modules", "dist"] } From d6bd4c6762829f65375581c14b397528e290e5fe Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 21 Apr 2026 16:41:15 +0200 Subject: [PATCH 5/8] fix: add debug logging for onPluginsReady lifecycle hook Log when the onPluginsReady hook starts and completes to aid debugging in development mode. Signed-off-by: MarioCadenas --- packages/appkit/src/core/appkit.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index 40fd1fcd8..607a15524 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -10,10 +10,13 @@ import type { } from "shared"; import { CacheManager } from "../cache"; import { ServiceContext } from "../context"; +import { createLogger } from "../logging/logger"; import { ResourceRegistry, ResourceType } from "../registry"; import type { TelemetryConfig } from "../telemetry"; import { TelemetryManager } from "../telemetry"; +const logger = createLogger("appkit"); + export class AppKit { #pluginInstances: Record = {}; #setupPromises: Promise[] = []; @@ -203,7 +206,11 @@ export class AppKit { const handle = instance as unknown as PluginMap; - await config.onPluginsReady?.(handle); + if (config.onPluginsReady) { + logger.debug("Running onPluginsReady hook"); + await config.onPluginsReady(handle); + logger.debug("onPluginsReady hook completed"); + } const serverPlugin = instance.#pluginInstances.server; if (serverPlugin && typeof (serverPlugin as any).start === "function") { From c03f8d02dcde8adaa4257d7801aadc890293766e Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 21 Apr 2026 17:38:59 +0200 Subject: [PATCH 6/8] fix: rename codemod reference to on-plugins-ready Update runtime detection error messages to point users to `npx appkit codemod on-plugins-ready` to match the hook name. Signed-off-by: MarioCadenas --- packages/appkit/src/plugins/server/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index 6c4841148..8ed13cea0 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -64,7 +64,7 @@ export class ServerPlugin extends Plugin { 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.", + "Run `npx appkit codemod on-plugins-ready --write` to auto-migrate.", ); } super(config); @@ -405,7 +405,7 @@ export class ServerPlugin extends Plugin { " appkit.server.extend(...);\n" + " },\n" + " });\n\n" + - "Run `npx appkit codemod customize-callback --write` to auto-migrate.", + "Run `npx appkit codemod on-plugins-ready --write` to auto-migrate.", ); }, }; From 30d7d3a0bac6d40f68a854692299efd71f9a4c23 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 21 Apr 2026 17:41:35 +0200 Subject: [PATCH 7/8] feat: add appkit codemod on-plugins-ready CLI Add `appkit codemod on-plugins-ready` command that auto-migrates old autoStart/extend/start patterns to the new onPluginsReady callback. Handles Pattern A (.then chain) and Pattern B (await + imperative). Bails with a warning for complex cases (non-server usage of appkit handle, multiple extend calls). Includes fixes from review: - Use raw slice offset for brace matching (not trimmed) - Use brace-aware parsing for .catch() handlers with arrow functions - Bail out when multiple .extend() calls detected in Pattern B Signed-off-by: MarioCadenas --- .../shared/src/cli/commands/codemod/index.ts | 17 + .../cli/commands/codemod/on-plugins-ready.ts | 478 ++++++++++++++++++ .../tests/fixtures/already-migrated.input.ts | 10 + .../autostart-true-with-port.input.ts | 5 + .../fixtures/pattern-a-arrow-catch.input.ts | 15 + .../fixtures/pattern-a-with-catch.input.ts | 15 + .../codemod/tests/fixtures/pattern-a.input.ts | 13 + .../tests/fixtures/pattern-b-complex.input.ts | 15 + .../fixtures/pattern-b-multi-extend.input.ts | 15 + .../codemod/tests/fixtures/pattern-b.input.ts | 13 + .../codemod/tests/on-plugins-ready.test.ts | 129 +++++ packages/shared/src/cli/index.ts | 2 + packages/shared/tsconfig.json | 2 +- 13 files changed, 728 insertions(+), 1 deletion(-) create mode 100644 packages/shared/src/cli/commands/codemod/index.ts create mode 100644 packages/shared/src/cli/commands/codemod/on-plugins-ready.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/already-migrated.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/autostart-true-with-port.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-arrow-catch.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a-with-catch.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-a.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-complex.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b-multi-extend.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/fixtures/pattern-b.input.ts create mode 100644 packages/shared/src/cli/commands/codemod/tests/on-plugins-ready.test.ts 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 000000000..2f9c160d5 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/index.ts @@ -0,0 +1,17 @@ +import { Command } from "commander"; +import { onPluginsReadyCommand } from "./on-plugins-ready"; + +/** + * Parent command for codemod operations. + * Subcommands: + * - on-plugins-ready: Migrate from autoStart/extend/start to onPluginsReady callback + */ +export const codemodCommand = new Command("codemod") + .description("Run codemods to migrate to newer AppKit APIs") + .addCommand(onPluginsReadyCommand) + .addHelpText( + "after", + ` +Examples: + $ appkit codemod on-plugins-ready --write`, + ); diff --git a/packages/shared/src/cli/commands/codemod/on-plugins-ready.ts b/packages/shared/src/cli/commands/codemod/on-plugins-ready.ts new file mode 100644 index 000000000..99f0d0b89 --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/on-plugins-ready.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 onPluginsReadyCommand = new Command("on-plugins-ready") + .description( + "Migrate createApp usage from autoStart/extend/start pattern to onPluginsReady callback", + ) + .option("--path ", "Path to the server entry file to migrate") + .option("--write", "Apply changes (default: dry-run)", false) + .addHelpText( + "after", + ` +Examples: + $ appkit codemod on-plugins-ready # dry-run, auto-detect files + $ appkit codemod on-plugins-ready --write # apply changes + $ appkit codemod on-plugins-ready --path server.ts # migrate a specific file`, + ) + .action(runCodemod); 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 000000000..ab1cf6d06 --- /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 000000000..96b70a4f8 --- /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 000000000..b6ae8c8a5 --- /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 000000000..faa04d5ef --- /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 000000000..73523d6a1 --- /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 000000000..c1fb25fab --- /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 000000000..dded09f27 --- /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 000000000..b56c00487 --- /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/commands/codemod/tests/on-plugins-ready.test.ts b/packages/shared/src/cli/commands/codemod/tests/on-plugins-ready.test.ts new file mode 100644 index 000000000..299e8f1de --- /dev/null +++ b/packages/shared/src/cli/commands/codemod/tests/on-plugins-ready.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 "../on-plugins-ready"; + +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/index.ts b/packages/shared/src/cli/index.ts index 71f09e6f6..4d0ed65b7 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 4a6e68b3e..5e195c3b6 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"] } From 3d50cfa6daca20f0e79f8b4d2e178470d2616527 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 21 Apr 2026 17:46:03 +0200 Subject: [PATCH 8/8] fix: handle async .then callbacks and full start() statement removal - Remove entire `await appkit.server.start();` statements instead of just stripping `.start()` (which left dangling `await appkit.server;`) - Detect async callbacks in .then() and emit `async onPluginsReady` so await expressions inside the body remain valid Signed-off-by: MarioCadenas --- .../src/cli/commands/codemod/on-plugins-ready.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/shared/src/cli/commands/codemod/on-plugins-ready.ts b/packages/shared/src/cli/commands/codemod/on-plugins-ready.ts index 99f0d0b89..37faeefe8 100644 --- a/packages/shared/src/cli/commands/codemod/on-plugins-ready.ts +++ b/packages/shared/src/cli/commands/codemod/on-plugins-ready.ts @@ -167,10 +167,12 @@ function migratePatternA(content: string): MigrationResult { let callbackBody = content.slice(bodyOpenBrace + 1, bodyCloseBrace).trim(); - // Remove .start() calls from the body + // Remove entire statements that are just .start() calls (e.g. `await appkit.server.start();`) callbackBody = callbackBody + .replace(/^\s*(?:await\s+)?\w+\.server\s*\.\s*start\(\s*\)\s*;?\s*$/gm, "") .replace(/\n\s*\.start\(\s*\)\s*;?/g, ";") .replace(/\.start\(\s*\)/g, "") + .replace(/\n\s*\n\s*\n/g, "\n\n") .trim(); // Clean up trailing semicolons @@ -180,6 +182,9 @@ function migratePatternA(content: string): MigrationResult { callbackBody += ";"; } + // Detect if the callback was async + const isAsync = /^async\s/.test(thenInner.trim()); + // Check for .catch() after .then(...) using brace-aware parsing const afterThenClose = content.slice(thenCloseParen + 1); const catchPatternMatch = afterThenClose.match(/^\s*(?:\)\s*)?\.catch\s*\(/); @@ -219,7 +224,8 @@ function migratePatternA(content: string): MigrationResult { .map((line) => ` ${line.trimStart()}`) .join("\n"); - const onPluginsReadyProp = `${needsComma}\n onPluginsReady(${paramName}) {\n${indentedBody}\n },`; + const asyncPrefix = isAsync ? "async " : ""; + const onPluginsReadyProp = `${needsComma}\n ${asyncPrefix}onPluginsReady(${paramName}) {\n${indentedBody}\n },`; const newConfig = `${beforeLastBrace}${onPluginsReadyProp}\n}`; // Build the replacement