Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(next): Update command doesn't update component code #1421

Merged
merged 13 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/loud-flies-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"shadcn-svelte": patch
---

fix: Ensure `utils.(js|ts)` is not fetched from the registry on `update` command
5 changes: 5 additions & 0 deletions .changeset/tricky-trains-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"shadcn-svelte": patch
---

fix: `update` command now properly updates components
18 changes: 14 additions & 4 deletions packages/cli/src/commands/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,28 @@ async function runUpdate(cwd: string, config: cliConfig.Config, options: UpdateO
if (p.isCancel(proceed) || proceed === false) cancel();
}

const tasks: p.Task[] = [];

const utils = selectedComponents.find((item) => item.name === "utils");
// `update utils` - update the utils.(ts|js) file
if (selectedComponents.find((item) => item.name === "utils")) {
if (utils) {
// remove utils so that it isn't fetched from the registry
selectedComponents = selectedComponents.filter((item) => item !== utils);

const extension = config.typescript ? ".ts" : ".js";
const utilsPath = config.resolvedPaths.utils + extension;

if (!existsSync(utilsPath)) {
throw error(`Failed to find ${highlight("utils")} at ${color.cyan(utilsPath)}`);
}

// utils.(ts|js) is not in the registry, it is a template, so we'll just overwrite it
await fs.writeFile(utilsPath, config.typescript ? UTILS : UTILS_JS);
tasks.push({
title: `Updating ${highlight(utilsPath)}`,
async task() {
// utils.(ts|js) is not in the registry, it is a template, so we'll just overwrite it
await fs.writeFile(utilsPath, config.typescript ? UTILS : UTILS_JS);
},
});
}

const tree = await registry.resolveTree({
Expand All @@ -173,7 +184,6 @@ async function runUpdate(cwd: string, config: cliConfig.Config, options: UpdateO

const componentsToRemove: Record<string, string[]> = {};
const dependencies = new Set<string>();
const tasks: p.Task[] = [];
for (const item of payload) {
const targetDir = registry.getItemTargetPath(config, item);
if (!targetDir) {
Expand Down
12 changes: 4 additions & 8 deletions packages/cli/src/utils/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,18 @@ export async function fetchTree(config: Config, tree: RegistryIndex) {

export function getItemTargetPath(
config: Config,
item: Pick<v.InferOutput<typeof schemas.registryItemWithContentSchema>, "type">,
item: v.InferOutput<typeof schemas.registryItemWithContentSchema>,
override?: string
) {
// Allow overrides for all items but ui.
if (override && item.type !== "registry:ui") {
return override;
}

const [parent, type] = item.type.split(":");
if (!parent || !type) return null;
const [, type] = item.type.split(":");
if (!type || !(type in config.resolvedPaths)) return null;

if (!(parent in config.resolvedPaths)) {
return null;
}

return path.join(config.resolvedPaths[parent as keyof typeof config.resolvedPaths], type);
return path.join(config.resolvedPaths[type as keyof typeof config.resolvedPaths]);
}

async function fetchRegistry(paths: string[]) {
Expand Down
94 changes: 94 additions & 0 deletions packages/cli/test/utils/registry/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { describe, expect, it } from "vitest";
import { getItemTargetPath } from "../../../src/utils/registry/index";
import { SITE_BASE_URL } from "../../../src/constants";

const config = {
style: "new-york",
tailwind: {
config: "tailwind.config.js",
css: "src/app.pcss",
baseColor: "zinc",
},
aliases: {
utils: "$lib/utils",
components: "$lib/components",
hooks: "$lib/hooks",
ui: "$lib/components/ui",
},
// not how they will look in the end but works for the tests
resolvedPaths: {
components: "./src/lib/components",
tailwindConfig: "./tailwind.config.js",
tailwindCss: "./src/app.pcss",
utils: "./src/lib/utils",
cwd: "./",
hooks: "./src/lib/hooks",
ui: "./src/lib/components/ui",
},
typescript: true,
registry: `${SITE_BASE_URL}/registry`,
};

describe("getItemTargetPath", () => {
it("returns null if invalid type missing `:`", async () => {
expect(
getItemTargetPath(config, {
name: "label",
dependencies: ["bits-ui@next"],
registryDependencies: [],
files: [
//... snip this since it doesn't matter
],
// @ts-expect-error Comes from over the wire in prod
type: "registry",
})
).toEqual(null);
});

it("returns null if `item.type` has invalid `:<type>`", async () => {
expect(
getItemTargetPath(config, {
name: "label",
dependencies: ["bits-ui@next"],
registryDependencies: [],
files: [
//... snip this since it doesn't matter
],
// @ts-expect-error Comes from over the wire in prod
type: "registry:foo",
})
).toEqual(null);
});

it("disallows overrides for `registry:ui`", async () => {
expect(
getItemTargetPath(
config,
{
name: "label",
dependencies: ["bits-ui@next"],
registryDependencies: [],
files: [
//... snip this since it doesn't matter
],
type: "registry:ui",
},
"./override-path"
)
).toEqual("src/lib/components/ui");
});

it("resolves item target path", async () => {
expect(
getItemTargetPath(config, {
name: "label",
dependencies: ["bits-ui@next"],
registryDependencies: [],
files: [
//... snip this since it doesn't matter
],
type: "registry:ui",
})
).toEqual("src/lib/components/ui");
});
});