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.

Sign up for GitHub

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

feat(cli): remove backup/restore/force options from set-version #1687

Merged
merged 4 commits into from
Oct 3, 2023
Merged

feat(cli): remove backup/restore/force options from set-version #1687

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d07743b
feat(cli): remove backup/restore/force options from set-version
holic Oct 3, 2023
f7defc0
Create lucky-clocks-love.md
holic Oct 3, 2023
a492eb5
expand note to include instructions on reverting to prev mud version
holic Oct 3, 2023
5a3f2bb
refactor to include all mud packages
holic Oct 3, 2023
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/lucky-clocks-love.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@latticexyz/cli": major
---

Removes `.mudbackup` file handling and `--backup`, `--restore, `--force` options from `mud set-version` command. Git will do a better job of this!
Copy link
Member

@alvrs alvrs Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add some quick instructions for how to do the same with git to the changelog?

e.g.

# restore all package.json to the version on <commit>
git checkout <commit> -- package.json && \
git checkout <commit> -- packages/contracts/package.json && \
git checkout <commit> -- packages/client/package.json

# restore all package.json the version on the `main` branch
git checkout main -- package.json && \
git checkout main -- packages/contracts/package.json && \
git checkout main -- packages/client/package.json

(Note i haven't actually tested these, we should make sure this works)

Now that i'm typing this out i wonder if git is actually doing a better job here? What if more packages changed in the meantime? (Is there another way to do this via git that i'm missing here?)

Copy link
Member Author

@holic holic Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately I don't really think it's our job to figure out a user's upgrade path and how they resolve diffs in their files (it's hard to get this right and not worth the complexity). In my mind, set-version is and should be just a shortcut for doing something like

pnpm up -r "@latticexyz/*@next"

except it takes into account all packages (not all of ours are prefixed with @latticexyz/) and has support for linking all packages too.

Edit: looks like this CLI doesn't support non-prefixed packages, but probably ought to

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the restore feature can be easily replaced with looking at the git history and then using pnpm mud set-version <prev-version> to reset it (especially since all MUD packages should have aligned versions). Still think that if we remove a feature we need to add instructions to the changeset for how to do the same with with the now recommended way (even if it's just "look up the previous version with git diff main package.json and run pnpm mud set-version <prev-version>")

46 changes: 3 additions & 43 deletions packages/cli/src/commands/set-version.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import chalk from "chalk";
import { existsSync, readFileSync, rmSync, writeFileSync } from "fs";
import { readFileSync, writeFileSync } from "fs";
import path from "path";
import type { CommandModule } from "yargs";
import { MUDError } from "@latticexyz/common/errors";
Expand All @@ -17,7 +17,6 @@ type Options = {
link?: string;
};

const BACKUP_FILE = ".mudbackup";
const MUD_PREFIX = "@latticexyz";

const commandModule: CommandModule<Options, Options> = {
Expand All @@ -27,12 +26,6 @@ const commandModule: CommandModule<Options, Options> = {

builder(yargs) {
return yargs.options({
backup: { type: "boolean", description: `Back up the current MUD versions to "${BACKUP_FILE}"` },
force: {
type: "boolean",
description: `Backup fails if a "${BACKUP_FILE}" file is found, unless --force is provided`,
},
restore: { type: "boolean", description: `Restore the previous MUD versions from "${BACKUP_FILE}"` },
mudVersion: { alias: "v", type: "string", description: "Set MUD to the given version" },
tag: {
alias: "t",
Expand Down Expand Up @@ -117,27 +110,11 @@ async function resolveVersion(options: Options) {
}

function updatePackageJson(filePath: string, options: Options): { workspaces?: string[] } {
const { restore, force, link } = options;
let { backup, mudVersion } = options;

const backupFilePath = path.join(path.dirname(filePath), BACKUP_FILE);
const backupFileExists = existsSync(backupFilePath);

// Create a backup file for previous MUD versions by default if linking to local MUD
if (link && !backupFileExists) backup = true;

// If `backup` is true and force not set, check if a backup file already exists and throw an error if it does
if (backup && !force && backupFileExists) {
throw new MUDError(
`A backup file already exists at ${backupFilePath}.\nUse --force to overwrite it or --restore to restore it.`
);
}
const { link } = options;
let { mudVersion } = options;

const packageJson = readPackageJson(filePath);

// Load .mudbackup if `restore` is true
const backupJson = restore ? readPackageJson(backupFilePath) : undefined;

// Find all MUD dependencies
const mudDependencies: Record<string, string> = {};
for (const key in packageJson.dependencies) {
Expand All @@ -154,15 +131,6 @@ function updatePackageJson(filePath: string, options: Options): { workspaces?: s
}
}

// Back up the current dependencies if `backup` is true
if (backup) {
writeFileSync(
backupFilePath,
JSON.stringify({ dependencies: mudDependencies, devDependencies: mudDevDependencies }, null, 2)
);
console.log(chalk.green(`Backed up MUD dependencies from ${filePath} to ${backupFilePath}`));
}

// Update the dependencies
for (const key in packageJson.dependencies) {
if (key.startsWith(MUD_PREFIX)) {
Expand All @@ -184,17 +152,9 @@ function updatePackageJson(filePath: string, options: Options): { workspaces?: s
logComparison(mudDependencies, packageJson.dependencies);
logComparison(mudDevDependencies, packageJson.devDependencies);

// Remove the backup file if `restore` is true and `backup` is false
// because the old backup file is no longer needed
if (restore && !backup) {
rmSync(backupFilePath);
console.log(chalk.green(`Cleaned up ${backupFilePath}`));
}

return packageJson;

function resolveMudVersion(key: string, type: "dependencies" | "devDependencies") {
if (restore && backupJson) return backupJson[type][key];
if (link) mudVersion = resolveLinkPath(filePath, link, key);
if (!mudVersion) return packageJson[type][key];
return mudVersion;
Expand Down