From 2c1864db10ce221b8c3df8f1aa173a86372dc112 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Tue, 17 Sep 2024 15:03:59 +0530 Subject: [PATCH 1/4] Add a transform for `createPromiseClient` -> `createClient` Signed-off-by: Sri Krishna Paritala --- .../src/migrations/v0.16.0-transform.spec.ts | 155 ++++++++++++++++++ .../src/migrations/v0.16.0-transform.ts | 145 ++++++++++++++++ 2 files changed, 300 insertions(+) create mode 100644 packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts create mode 100644 packages/connect-migrate/src/migrations/v0.16.0-transform.ts diff --git a/packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts b/packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts new file mode 100644 index 000000000..a90fe53f5 --- /dev/null +++ b/packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts @@ -0,0 +1,155 @@ +// Copyright 2021-2024 The Connect Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import jscodeshift from "jscodeshift"; +import transform from "./v0.16.0-transform"; + +function t( + source: string, + parser: "tsx" | "babel" | "ts" | "babylon" | "flow" = "tsx", +) { + const shift = jscodeshift.withParser(parser); + return transform( + { path: "test-file", source }, + { + jscodeshift: shift, + j: shift, + stats: () => {}, + report: () => {}, + }, + {}, + ); +} + +describe("rename symbols using", () => { + describe("'import' with", () => { + it("local identifier", () => { + const got = ` + import { createPromiseClient as create } from "@connectrpc/connect"; + const createPromiseClient = create; + `; + const want = ` + import { createClient as create } from "@connectrpc/connect"; + const createPromiseClient = create; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("identifier", () => { + const got = ` + import { createPromiseClient } from "@connectrpc/connect"; + const a = createPromiseClient; + console.log(createPromiseClient); + const c = createPromiseClient(); + type R = ReturnType; + `; + const want = ` + import { createClient } from "@connectrpc/connect"; + const a = createClient; + console.log(createClient); + const c = createClient(); + type R = ReturnType; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("namespace", () => { + const got = ` + import * as connect from "@connectrpc/connect"; + const a = connect.createPromiseClient; + console.log(connect.createPromiseClient); + const c = connect.createPromiseClient(); + type R = ReturnType; + `; + const want = ` + import * as connect from "@connectrpc/connect"; + const a = connect.createClient; + console.log(connect.createClient); + const c = connect.createClient(); + type R = ReturnType; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("default", () => { + const got = ` + import connect from "@connectrpc/connect"; + const a = connect.createPromiseClient; + console.log(connect.createPromiseClient); + const c = connect.createPromiseClient(); + type R = ReturnType; + `; + const want = ` + import connect from "@connectrpc/connect"; + const a = connect.createClient; + console.log(connect.createClient); + const c = connect.createClient(); + type R = ReturnType; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + }); + describe("'require' with", () => { + it("const", () => { + const got = ` + const connect = require("@connectrpc/connect"); + const a = connect.createPromiseClient; + console.log(connect.createPromiseClient); + const c = connect.createPromiseClient(); + type R = ReturnType; + `; + const want = ` + const connect = require("@connectrpc/connect"); + const a = connect.createClient; + console.log(connect.createClient); + const c = connect.createClient(); + type R = ReturnType; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("spread", () => { + const got = ` + const { createPromiseClient } = require("@connectrpc/connect"); + const a = createPromiseClient; + console.log(createPromiseClient); + const c = createPromiseClient(); + type R = ReturnType; + `; + const want = ` + const { createClient } = require("@connectrpc/connect"); + const a = createClient; + console.log(createClient); + const c = createClient(); + type R = ReturnType; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("let", () => { + const got = ` + let connect; + connect = require("@connectrpc/connect"); + const a = connect.createPromiseClient; + console.log(connect.createPromiseClient); + const c = connect.createPromiseClient(); + type R = ReturnType; + `; + const want = ` + let connect; + connect = require("@connectrpc/connect"); + const a = connect.createClient; + console.log(connect.createClient); + const c = connect.createClient(); + type R = ReturnType; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + }); +}); diff --git a/packages/connect-migrate/src/migrations/v0.16.0-transform.ts b/packages/connect-migrate/src/migrations/v0.16.0-transform.ts new file mode 100644 index 000000000..d7b30a852 --- /dev/null +++ b/packages/connect-migrate/src/migrations/v0.16.0-transform.ts @@ -0,0 +1,145 @@ +// Copyright 2021-2024 The Connect Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import j from "jscodeshift"; + +const importPath = "@connectrpc/connect"; +const fromFunction = "createPromiseClient"; +const toFunction = "createClient"; + +/** + * This transform handles changing all usages of `createPromiseClient` to `createClient`. + */ +const transform: j.Transform = (file, { j }, options) => { + const root = j(file.source); + // import { createPromiseClient [as ]) } from "@connectrpc/connect"; + root + .find(j.ImportDeclaration, { + source: { value: importPath }, + specifiers: [ + { type: "ImportSpecifier", imported: { name: fromFunction } }, + ], + }) + .forEach((path) => { + path.value.specifiers?.forEach((s) => { + s = s as j.ImportSpecifier; + // import { createPromiseClient as } from "@connectrpc/connect"; + // + // We should just rename createPromiseClient here and user code will continue to use local. + if (s.local?.loc !== s.imported.loc) { + s.imported.name = toFunction; + return; + } + replace(root); + }); + }); + // import * as connect from "@connectrpc/connect"; + // import connect from "@connectrpc/connect"; + root + .find(j.ImportDeclaration, { + source: { value: importPath }, + specifiers: (s) => + s?.some( + (v) => + v.type === "ImportNamespaceSpecifier" || + v.type === "ImportDefaultSpecifier", + ) ?? false, + }) + .forEach((path) => { + path.value.specifiers?.forEach((s) => { + s = s as j.ImportNamespaceSpecifier | j.ImportDefaultSpecifier; + const qualifier = s.local?.name; + if (qualifier === undefined) { + return; + } + replace(root, qualifier); + }); + }); + // require("@connectrpc/connect") + const requireCall = { + callee: { + type: "Identifier", + name: "require", + }, + arguments: [ + { + type: "StringLiteral", + value: importPath, + }, + ], + } as const; + // const ... = require("@connectrpc/connect"); + root.find(j.VariableDeclarator, { init: requireCall }).forEach((path) => { + // const connect = require("@connectrpc/connect"); + if (path.value.id.type === "Identifier") { + replace(root, path.value.id.name); + return; + } + // const { createPromiseClient[:local] } = require("@connectrpc/connect"); + if (path.value.id.type === "ObjectPattern") { + const property = path.value.id.properties.find( + (p) => + p.type === "ObjectProperty" && + p.key.type === "Identifier" && + p.key.name === fromFunction, + ) as (j.ObjectProperty & { key: j.Identifier }) | undefined; + if (property !== undefined) { + if (property.value.loc?.start === property.key.loc?.start) { + // const { createPromiseClient } = require("@connectrpc/connect"); + replace(root); + } else { + // const { createPromiseClient: local } = require("@connectrpc/connect"); + property.key.name = toFunction; + } + } + } + }); + // let connect; + // connect = require("@connectrpc/connect"); + root.find(j.AssignmentExpression, { right: requireCall }).forEach((path) => { + if (path.value.left.type === "Identifier") { + replace(root, path.value.left.name); + return; + } + }); + return root.toSource(options); +}; + +function replace(root: j.Collection, qualifier?: string) { + if (qualifier === undefined) { + root + .find(j.Identifier, { name: fromFunction }) + .forEach((path) => (path.value.name = toFunction)); + } + // connect.createPromiseClient + root + .find(j.MemberExpression, { + object: { type: "Identifier", name: qualifier }, + property: { type: "Identifier", name: fromFunction }, + }) + .forEach((path) => { + (path.value.property as j.Identifier).name = toFunction; + }); + // typeof connect.createPromiseClient + root + .find(j.TSQualifiedName, { + left: { type: "Identifier", name: qualifier }, + right: { type: "Identifier", name: fromFunction }, + }) + .forEach((path) => { + (path.value.right as j.Identifier).name = toFunction; + }); +} + +export default transform; From c3000f682c7fc48239fe17bda929474e7aa16ead Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Wed, 18 Sep 2024 12:20:59 +0530 Subject: [PATCH 2/4] Rename spec Signed-off-by: Sri Krishna Paritala --- packages/connect-migrate/src/cli.ts | 23 +++--- ...form.spec.ts => v1.16.0-transform.spec.ts} | 2 +- ...16.0-transform.ts => v1.16.0-transform.ts} | 46 ++++++++---- .../src/migrations/v1.16.0.spec.ts | 22 ++++++ .../connect-migrate/src/migrations/v1.16.0.ts | 72 +++++++++++++++++++ 5 files changed, 140 insertions(+), 25 deletions(-) rename packages/connect-migrate/src/migrations/{v0.16.0-transform.spec.ts => v1.16.0-transform.spec.ts} (99%) rename packages/connect-migrate/src/migrations/{v0.16.0-transform.ts => v1.16.0-transform.ts} (76%) create mode 100644 packages/connect-migrate/src/migrations/v1.16.0.spec.ts create mode 100644 packages/connect-migrate/src/migrations/v1.16.0.ts diff --git a/packages/connect-migrate/src/cli.ts b/packages/connect-migrate/src/cli.ts index 96aae877e..6261e9f00 100644 --- a/packages/connect-migrate/src/cli.ts +++ b/packages/connect-migrate/src/cli.ts @@ -19,6 +19,7 @@ import { parseCommandLineArgs } from "./arguments"; import { scan } from "./lib/scan"; import { Logger } from "./lib/logger"; import { v0_13_1 } from "./migrations/v0.13.1"; +import { v1_16_0 } from "./migrations/v1.16.0"; const usage = `USAGE: connect-migrate [flags] Updates references to connect-es packages in your project to use @connectrpc. @@ -47,17 +48,19 @@ async function main() { if (args.help) { exitOk(usage); } - const scanned = scan(args.ignorePatterns, process.cwd(), print, logger); - if (!scanned.ok) { - return exitErr(scanned.errorMessage, false); - } - if (v0_13_1.applicable(scanned)) { - const result = v0_13_1.migrate({ scanned, args, print, logger }); - if (!result.ok) { - return exitErr(result.errorMessage, result.dumpLogfile ?? false); + for (const migration of [v0_13_1, v1_16_0]) { + const scanned = scan(args.ignorePatterns, process.cwd(), print, logger); + if (!scanned.ok) { + return exitErr(scanned.errorMessage, false); + } + if (migration.applicable(scanned)) { + const result = migration.migrate({ scanned, args, print, logger }); + if (!result.ok) { + return exitErr(result.errorMessage, result.dumpLogfile ?? false); + } + } else { + exitOk("It looks like you are already up to date 🎉"); } - } else { - exitOk("It looks like you are already up to date 🎉"); } } catch (e) { logger.error(`caught error: ${String(e)}`); diff --git a/packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts b/packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts similarity index 99% rename from packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts rename to packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts index a90fe53f5..b32df98de 100644 --- a/packages/connect-migrate/src/migrations/v0.16.0-transform.spec.ts +++ b/packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts @@ -13,7 +13,7 @@ // limitations under the License. import jscodeshift from "jscodeshift"; -import transform from "./v0.16.0-transform"; +import transform from "./v1.16.0-transform"; function t( source: string, diff --git a/packages/connect-migrate/src/migrations/v0.16.0-transform.ts b/packages/connect-migrate/src/migrations/v1.16.0-transform.ts similarity index 76% rename from packages/connect-migrate/src/migrations/v0.16.0-transform.ts rename to packages/connect-migrate/src/migrations/v1.16.0-transform.ts index d7b30a852..43fedfe04 100644 --- a/packages/connect-migrate/src/migrations/v0.16.0-transform.ts +++ b/packages/connect-migrate/src/migrations/v1.16.0-transform.ts @@ -17,6 +17,8 @@ import j from "jscodeshift"; const importPath = "@connectrpc/connect"; const fromFunction = "createPromiseClient"; const toFunction = "createClient"; +const fromType = "PromiseClient"; +const toType = "Client"; /** * This transform handles changing all usages of `createPromiseClient` to `createClient`. @@ -28,7 +30,10 @@ const transform: j.Transform = (file, { j }, options) => { .find(j.ImportDeclaration, { source: { value: importPath }, specifiers: [ - { type: "ImportSpecifier", imported: { name: fromFunction } }, + { + type: "ImportSpecifier", + imported: { name: (name) => [fromFunction, fromType].includes(name) }, + }, ], }) .forEach((path) => { @@ -38,10 +43,14 @@ const transform: j.Transform = (file, { j }, options) => { // // We should just rename createPromiseClient here and user code will continue to use local. if (s.local?.loc !== s.imported.loc) { - s.imported.name = toFunction; + s.imported.name = s.imported.name === fromType ? toType : toFunction; return; } - replace(root); + replace( + root, + s.imported.name, + s.imported.name === fromType ? toType : toFunction, + ); }); }); // import * as connect from "@connectrpc/connect"; @@ -63,7 +72,8 @@ const transform: j.Transform = (file, { j }, options) => { if (qualifier === undefined) { return; } - replace(root, qualifier); + replace(root, fromType, toType, qualifier); + replace(root, fromFunction, toFunction, qualifier); }); }); // require("@connectrpc/connect") @@ -83,7 +93,8 @@ const transform: j.Transform = (file, { j }, options) => { root.find(j.VariableDeclarator, { init: requireCall }).forEach((path) => { // const connect = require("@connectrpc/connect"); if (path.value.id.type === "Identifier") { - replace(root, path.value.id.name); + replace(root, fromType, toType, path.value.id.name); + replace(root, fromFunction, toFunction, path.value.id.name); return; } // const { createPromiseClient[:local] } = require("@connectrpc/connect"); @@ -97,7 +108,7 @@ const transform: j.Transform = (file, { j }, options) => { if (property !== undefined) { if (property.value.loc?.start === property.key.loc?.start) { // const { createPromiseClient } = require("@connectrpc/connect"); - replace(root); + replace(root, fromFunction, toFunction); } else { // const { createPromiseClient: local } = require("@connectrpc/connect"); property.key.name = toFunction; @@ -109,36 +120,43 @@ const transform: j.Transform = (file, { j }, options) => { // connect = require("@connectrpc/connect"); root.find(j.AssignmentExpression, { right: requireCall }).forEach((path) => { if (path.value.left.type === "Identifier") { - replace(root, path.value.left.name); + replace(root, fromType, toType, path.value.left.name); + replace(root, fromFunction, toFunction, path.value.left.name); return; } }); return root.toSource(options); }; -function replace(root: j.Collection, qualifier?: string) { +function replace( + root: j.Collection, + from: string, + to: string, + qualifier?: string, +) { if (qualifier === undefined) { root - .find(j.Identifier, { name: fromFunction }) - .forEach((path) => (path.value.name = toFunction)); + .find(j.Identifier, { name: from }) + .forEach((path) => (path.value.name = to)); + return; } // connect.createPromiseClient root .find(j.MemberExpression, { object: { type: "Identifier", name: qualifier }, - property: { type: "Identifier", name: fromFunction }, + property: { type: "Identifier", name: from }, }) .forEach((path) => { - (path.value.property as j.Identifier).name = toFunction; + (path.value.property as j.Identifier).name = to; }); // typeof connect.createPromiseClient root .find(j.TSQualifiedName, { left: { type: "Identifier", name: qualifier }, - right: { type: "Identifier", name: fromFunction }, + right: { type: "Identifier", name: from }, }) .forEach((path) => { - (path.value.right as j.Identifier).name = toFunction; + (path.value.right as j.Identifier).name = to; }); } diff --git a/packages/connect-migrate/src/migrations/v1.16.0.spec.ts b/packages/connect-migrate/src/migrations/v1.16.0.spec.ts new file mode 100644 index 000000000..34e136059 --- /dev/null +++ b/packages/connect-migrate/src/migrations/v1.16.0.spec.ts @@ -0,0 +1,22 @@ +// Copyright 2021-2024 The Connect Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { validRange } from "semver"; +import { transformOnly } from "./v1.16.0"; + +describe("transformOnly", function () { + it("should have valid range", () => { + expect(validRange(transformOnly.from.range)).not.toBeNull(); + }); +}); diff --git a/packages/connect-migrate/src/migrations/v1.16.0.ts b/packages/connect-migrate/src/migrations/v1.16.0.ts new file mode 100644 index 000000000..fdcd239a0 --- /dev/null +++ b/packages/connect-migrate/src/migrations/v1.16.0.ts @@ -0,0 +1,72 @@ +// Copyright 2021-2024 The Connect Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { Scanned } from "../lib/scan"; +import replaceCalls from "./v1.16.0-transform"; +import { MigrateError, MigrateSuccess, Migration } from "../migration"; +import { + DependencyReplacement, + replaceDependencies, +} from "../lib/replace-dependencies"; +import { updateSourceFile } from "../lib/update-source-file"; +import { migrateSourceFiles } from "../lib/migrate-source-files"; + +/** + * We want to limit the transformation to v1. + */ +export const transformOnly = { + from: { name: "@connectrpc/connect", range: "^1.16.0" }, + to: { version: "^1.16.0" }, +} satisfies DependencyReplacement; + +/** + * Migrates code to use new symbols `createClient` and `Client` instead + * of `createPromiseClient` and `PromiseClient`. + */ +export const v1_16_0: Migration = { + applicable(scanned: Scanned) { + return scanned.packageFiles.some( + ({ pkg }) => replaceDependencies(pkg, [transformOnly]) != null, + ); + }, + migrate({ + scanned, + print, + logger, + updateSourceFileFn = updateSourceFile, + }): MigrateError | MigrateSuccess { + const { sourceFileErrors } = migrateSourceFiles( + scanned, + replaceCalls, + print, + logger, + updateSourceFileFn, + ); + if (sourceFileErrors > 0) { + return { + ok: false, + errorMessage: [ + `⚠️${sourceFileErrors} source ${ + sourceFileErrors == 1 ? "file" : "files" + } could not be updated.`, + `You may have to update the files manually. Check the log for details.`, + ].join("\n"), + dumpLogfile: true, + }; + } + return { + ok: true, + }; + }, +}; From 2f3fc60bbdda824fa223b60dac9cb8708f033b70 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Wed, 18 Sep 2024 15:10:12 +0530 Subject: [PATCH 3/4] Fix order Signed-off-by: Sri Krishna Paritala --- packages/connect-migrate/src/cli.ts | 20 ++-- .../src/lib/replace-dependencies.ts | 17 ++- .../connect-migrate/src/migrations/v0.13.1.ts | 12 ++- .../src/migrations/v1.16.0-transform.spec.ts | 52 +++++++++ .../src/migrations/v1.16.0.spec.ts | 100 +++++++++++++++++- .../connect-migrate/src/migrations/v1.16.0.ts | 68 ++++++++---- 6 files changed, 224 insertions(+), 45 deletions(-) diff --git a/packages/connect-migrate/src/cli.ts b/packages/connect-migrate/src/cli.ts index 6261e9f00..041ad907e 100644 --- a/packages/connect-migrate/src/cli.ts +++ b/packages/connect-migrate/src/cli.ts @@ -20,6 +20,7 @@ import { scan } from "./lib/scan"; import { Logger } from "./lib/logger"; import { v0_13_1 } from "./migrations/v0.13.1"; import { v1_16_0 } from "./migrations/v1.16.0"; +import type { Migration } from "./migration"; const usage = `USAGE: connect-migrate [flags] Updates references to connect-es packages in your project to use @connectrpc. @@ -33,7 +34,7 @@ Flags: `; const logger = new Logger(); - +const migrations = [v0_13_1, v1_16_0]; void main(); async function main() { @@ -48,20 +49,23 @@ async function main() { if (args.help) { exitOk(usage); } - for (const migration of [v0_13_1, v1_16_0]) { - const scanned = scan(args.ignorePatterns, process.cwd(), print, logger); - if (!scanned.ok) { - return exitErr(scanned.errorMessage, false); - } + const scanned = scan(args.ignorePatterns, process.cwd(), print, logger); + if (!scanned.ok) { + return exitErr(scanned.errorMessage, false); + } + const applied: Migration[] = []; + for (const migration of migrations) { if (migration.applicable(scanned)) { const result = migration.migrate({ scanned, args, print, logger }); if (!result.ok) { return exitErr(result.errorMessage, result.dumpLogfile ?? false); } - } else { - exitOk("It looks like you are already up to date 🎉"); + applied.push(migration); } } + if (applied.length == 0) { + exitOk("It looks like you are already up to date 🎉"); + } } catch (e) { logger.error(`caught error: ${String(e)}`); if (e instanceof Error && e.stack !== undefined) { diff --git a/packages/connect-migrate/src/lib/replace-dependencies.ts b/packages/connect-migrate/src/lib/replace-dependencies.ts index b638f156c..5cf2cdebe 100644 --- a/packages/connect-migrate/src/lib/replace-dependencies.ts +++ b/packages/connect-migrate/src/lib/replace-dependencies.ts @@ -27,12 +27,11 @@ export type DependencyReplacement = { * Replace all dependencies matching "from" with "to". */ export function replaceDependencies( - pkg: Readonly, + pkg: PackageJson, replacements: DependencyReplacement[], ): PackageJson | null { const modifiedPackageNames = new Set(); const replacedPackageNames = new Map(); - const copy = clonePackageJson(pkg); // replace dependencies for (const replacement of replacements) { for (const p of [ @@ -40,7 +39,7 @@ export function replaceDependencies( "devDependencies", "peerDependencies", ] as const) { - const deps = copy[p] ?? {}; + const deps = pkg[p] ?? {}; for (const [packageName, versionRange] of Object.entries(deps)) { if (packageName !== replacement.from.name) { continue; @@ -72,14 +71,14 @@ export function replaceDependencies( } // replace bundled dependencies, but only for package names we replaced for (const p of ["bundleDependencies", "bundledDependencies"] as const) { - const bundled = copy[p]; + const bundled = pkg[p]; if (!Array.isArray(bundled)) { continue; } - copy[p] = bundled.map((n) => replacedPackageNames.get(n) ?? n); + pkg[p] = bundled.map((n) => replacedPackageNames.get(n) ?? n); } // replace peer dependency meta, but only for package names we replaced - const meta = copy.peerDependenciesMeta; + const meta = pkg.peerDependenciesMeta; if (meta !== undefined) { for (const [n, value] of Object.entries(meta)) { const newName = replacedPackageNames.get(n); @@ -90,9 +89,5 @@ export function replaceDependencies( meta[newName] = value; } } - return modifiedPackageNames.size > 0 ? copy : null; -} - -function clonePackageJson(pkg: PackageJson): PackageJson { - return JSON.parse(JSON.stringify(pkg)) as PackageJson; + return modifiedPackageNames.size > 0 ? pkg : null; } diff --git a/packages/connect-migrate/src/migrations/v0.13.1.ts b/packages/connect-migrate/src/migrations/v0.13.1.ts index 0543a7fc1..0b5bbd5d4 100644 --- a/packages/connect-migrate/src/migrations/v0.13.1.ts +++ b/packages/connect-migrate/src/migrations/v0.13.1.ts @@ -132,7 +132,9 @@ export const dependencyReplacements: DependencyReplacement[] = [ export const v0_13_1: Migration = { applicable(scanned: Scanned) { return scanned.packageFiles.some( - ({ pkg }) => replaceDependencies(pkg, dependencyReplacements) !== null, + ({ pkg }) => + replaceDependencies(structuredClone(pkg), dependencyReplacements) !== + null, ); }, migrate({ @@ -149,7 +151,9 @@ export const v0_13_1: Migration = { const oldPluginUsed = scanned.packageFiles .filter( ({ pkg }) => - replaceDependencies(pkg, [oldPluginReplacement]) !== null, + replaceDependencies(structuredClone(pkg), [ + oldPluginReplacement, + ]) !== null, ) .map(({ path }) => path); if (oldPluginUsed.length > 0) { @@ -163,7 +167,9 @@ export const v0_13_1: Migration = { const removedWebExportsUsed = scanned.packageFiles .filter( ({ pkg }) => - replaceDependencies(pkg, [removedWebExportReplacement]) !== null, + replaceDependencies(structuredClone(pkg), [ + removedWebExportReplacement, + ]) !== null, ) .map(({ path }) => path); if (removedWebExportsUsed.length > 0) { diff --git a/packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts b/packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts index b32df98de..6fe1a966c 100644 --- a/packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts +++ b/packages/connect-migrate/src/migrations/v1.16.0-transform.spec.ts @@ -34,6 +34,58 @@ function t( describe("rename symbols using", () => { describe("'import' with", () => { + describe("for type", () => { + describe("local identifier", () => { + it("plain", () => { + const got = ` + import { PromiseClient as client } from "@connectrpc/connect"; + type PromiseClient = client; + `; + const want = ` + import { Client as client } from "@connectrpc/connect"; + type PromiseClient = client; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("type qualified", () => { + const got = ` + import { type PromiseClient as client } from "@connectrpc/connect"; + type PromiseClient = client; + `; + const want = ` + import { type Client as client } from "@connectrpc/connect"; + type PromiseClient = client; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + it("type only", () => { + const got = ` + import type { PromiseClient as client } from "@connectrpc/connect"; + type PromiseClient = client; + `; + const want = ` + import type { Client as client } from "@connectrpc/connect"; + type PromiseClient = client; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + }); + describe("identifier", () => { + it("plain", () => { + const got = ` + import { PromiseClient } from "@connectrpc/connect"; + type a = PromiseClient; + type R = Readonly; + `; + const want = ` + import { Client } from "@connectrpc/connect"; + type a = Client; + type R = Readonly; + `; + expect(t(got)?.trim()).toBe(want.trim()); + }); + }); + }); it("local identifier", () => { const got = ` import { createPromiseClient as create } from "@connectrpc/connect"; diff --git a/packages/connect-migrate/src/migrations/v1.16.0.spec.ts b/packages/connect-migrate/src/migrations/v1.16.0.spec.ts index 34e136059..119bee5f0 100644 --- a/packages/connect-migrate/src/migrations/v1.16.0.spec.ts +++ b/packages/connect-migrate/src/migrations/v1.16.0.spec.ts @@ -12,11 +12,101 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { validRange } from "semver"; -import { transformOnly } from "./v1.16.0"; +import { v1_16_0 } from "./v1.16.0"; +import type { PackageJson } from "../lib/package-json"; +import type { MigrateOptions } from "../migration"; -describe("transformOnly", function () { - it("should have valid range", () => { - expect(validRange(transformOnly.from.range)).not.toBeNull(); +describe("migration", function () { + const packageJsonWritten: { path: string; pkg: PackageJson }[] = []; + const lockFilesUpdated: string[] = []; + let opt: MigrateOptions; + beforeEach(function () { + packageJsonWritten.splice(0); + lockFilesUpdated.splice(0); + opt = { + scanned: { + ok: true, + lockFiles: ["package-lock.json"], + sourceFiles: [], + packageFiles: [], + }, + args: { + ok: true, + help: false, + version: false, + ignorePatterns: [], + noInstall: false, + forceUpdate: false, + }, + print: () => { + // + }, + updateSourceFileFn: () => ({ + ok: true, + modified: false, + }), + writePackageJsonFileFn: (path: string, pkg: PackageJson) => + packageJsonWritten.push({ path, pkg }), + runInstallFn: (lockfilePath) => { + lockFilesUpdated.push(lockfilePath); + return true; + }, + }; + }); + describe("should be applicable", function () { + it("for 1.16.0", () => { + opt.scanned.packageFiles = [ + { + path: "package.json", + pkg: { + dependencies: { + "@connectrpc/connect": "^1.16.0", + }, + }, + }, + ]; + expect(v1_16_0.applicable(opt.scanned)).toBeTrue(); + }); + it("after 1.16.0", () => { + opt.scanned.packageFiles = [ + { + path: "package.json", + pkg: { + dependencies: { + "@connectrpc/connect": "^1.17.0", + }, + }, + }, + ]; + expect(v1_16_0.applicable(opt.scanned)).toBeTrue(); + }); + }); + describe("should not be applicable", function () { + it("before 1.16.0", () => { + opt.scanned.packageFiles = [ + { + path: "package.json", + pkg: { + dependencies: { + "@connectrpc/connect": "^1.15.0", + }, + }, + }, + ]; + expect(v1_16_0.applicable(opt.scanned)).toBeFalse(); + }); + it("from 2.0.0", () => { + opt.scanned.packageFiles = [ + { + path: "package.json", + pkg: { + dependencies: { + "@connectrpc/connect": "^2.0.0", + }, + }, + }, + ]; + expect(v1_16_0.applicable(opt.scanned)).toBeFalse(); + }); }); }); diff --git a/packages/connect-migrate/src/migrations/v1.16.0.ts b/packages/connect-migrate/src/migrations/v1.16.0.ts index fdcd239a0..42b142386 100644 --- a/packages/connect-migrate/src/migrations/v1.16.0.ts +++ b/packages/connect-migrate/src/migrations/v1.16.0.ts @@ -12,23 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Scanned } from "../lib/scan"; +import type { Scanned } from "../lib/scan"; import replaceCalls from "./v1.16.0-transform"; -import { MigrateError, MigrateSuccess, Migration } from "../migration"; -import { - DependencyReplacement, - replaceDependencies, -} from "../lib/replace-dependencies"; +import type { MigrateError, MigrateSuccess, Migration } from "../migration"; import { updateSourceFile } from "../lib/update-source-file"; import { migrateSourceFiles } from "../lib/migrate-source-files"; - -/** - * We want to limit the transformation to v1. - */ -export const transformOnly = { - from: { name: "@connectrpc/connect", range: "^1.16.0" }, - to: { version: "^1.16.0" }, -} satisfies DependencyReplacement; +import * as semver from "semver"; +import { dirname } from "node:path"; /** * Migrates code to use new symbols `createClient` and `Client` instead @@ -36,9 +26,7 @@ export const transformOnly = { */ export const v1_16_0: Migration = { applicable(scanned: Scanned) { - return scanned.packageFiles.some( - ({ pkg }) => replaceDependencies(pkg, [transformOnly]) != null, - ); + return getMatchingPackages(scanned.packageFiles).length > 0; }, migrate({ scanned, @@ -46,8 +34,24 @@ export const v1_16_0: Migration = { logger, updateSourceFileFn = updateSourceFile, }): MigrateError | MigrateSuccess { + // We want to limit to only matched packages. + const matchingPackages = getMatchingPackages(scanned.packageFiles); + const matchingSources = []; + for (const source of scanned.sourceFiles) { + // If source is within the package directory. + if ( + matchingPackages.some(({ path }) => source.startsWith(dirname(path))) + ) { + matchingSources.push(source); + } + } + if (matchingSources.length === 0) { + return { + ok: true, + }; + } const { sourceFileErrors } = migrateSourceFiles( - scanned, + { ...scanned, sourceFiles: matchingSources }, replaceCalls, print, logger, @@ -70,3 +74,31 @@ export const v1_16_0: Migration = { }; }, }; + +function getMatchingPackages(packageFiles: Scanned["packageFiles"]) { + const matched: Scanned["packageFiles"] = []; + for (const packageFile of packageFiles) { + if ( + [ + packageFile.pkg.dependencies, + packageFile.pkg.devDependencies, + packageFile.pkg.peerDependencies, + ] + .filter((e) => e !== undefined) + .flatMap((deps) => Object.entries(deps)) + .some(([packageName, versionRange]) => { + if (packageName !== "@connectrpc/connect") { + return false; + } + const minVersion = semver.minVersion(versionRange); + if (minVersion === null) { + return false; + } + return semver.satisfies(minVersion, "^1.16.0"); + }) + ) { + matched.push(packageFile); + } + } + return matched; +} From 66ab073bc67861c3f9d3600bf9f161f0aebe1717 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Wed, 18 Sep 2024 16:57:26 +0530 Subject: [PATCH 4/4] Remove file filter Signed-off-by: Sri Krishna Paritala --- .../connect-migrate/src/migrations/v1.16.0.ts | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/packages/connect-migrate/src/migrations/v1.16.0.ts b/packages/connect-migrate/src/migrations/v1.16.0.ts index 42b142386..afd60da06 100644 --- a/packages/connect-migrate/src/migrations/v1.16.0.ts +++ b/packages/connect-migrate/src/migrations/v1.16.0.ts @@ -18,7 +18,6 @@ import type { MigrateError, MigrateSuccess, Migration } from "../migration"; import { updateSourceFile } from "../lib/update-source-file"; import { migrateSourceFiles } from "../lib/migrate-source-files"; import * as semver from "semver"; -import { dirname } from "node:path"; /** * Migrates code to use new symbols `createClient` and `Client` instead @@ -34,24 +33,8 @@ export const v1_16_0: Migration = { logger, updateSourceFileFn = updateSourceFile, }): MigrateError | MigrateSuccess { - // We want to limit to only matched packages. - const matchingPackages = getMatchingPackages(scanned.packageFiles); - const matchingSources = []; - for (const source of scanned.sourceFiles) { - // If source is within the package directory. - if ( - matchingPackages.some(({ path }) => source.startsWith(dirname(path))) - ) { - matchingSources.push(source); - } - } - if (matchingSources.length === 0) { - return { - ok: true, - }; - } const { sourceFileErrors } = migrateSourceFiles( - { ...scanned, sourceFiles: matchingSources }, + scanned, replaceCalls, print, logger,