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

feat(create-remix): add --overwrite flag #7062

Merged
merged 17 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions .changeset/create-remix-override.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"create-remix": minor
---

Remove empty directory checking in favor of `overwrite` prompt/flag.

`create-remix` now allows you to write into an existing non-empty directory. It will perform a file-level comparison and if the template will overwrite any existing files in the destination directory, it will prompt you if it's OK to overwrite the files in the destination directory. If you answer no (the default) then it will exit without copying any files into the destination directory. You may skip this prompt with the `--overwrite CLI flag.
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions docs/other-api/create-remix.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,8 @@ To create a new project from a template in a private GitHub repo, pass the `--to
</docs-info>
</aside>

### `create-remix --overwrite`

If `create-remix` detects any file collisions between the template and the directory you are creating your app in, it will prompt you for confirmation that it's OK to overwrite those files with the template versions. You may skip this prompt with the `--overwrite` CLI flag.

[templates]: ../pages/templates
162 changes: 76 additions & 86 deletions packages/create-remix/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import process from "node:process";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import fse from "fs-extra";
import stripAnsi from "strip-ansi";
import rm from "rimraf";
import execa from "execa";
Expand All @@ -15,11 +17,11 @@ import {
ensureDirectory,
error,
fileExists,
getDirectoryFilesRecursive,
info,
isInteractive,
isValidJsonObject,
log,
pathContains,
sleep,
strip,
success,
Expand All @@ -43,7 +45,8 @@ async function createRemix(argv: string[]) {
let steps = [
introStep,
projectNameStep,
templateStep,
copyTemplateToTempDirStep,
copyTempDirToAppDirStep,
Comment on lines +51 to +52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to do the file level comparison, we now extract into a temp directory and then compare the temp dir to the destination dir.

gitInitQuestionStep,
installDependenciesQuestionStep,
runInitScriptQuestionStep,
Expand Down Expand Up @@ -89,6 +92,7 @@ async function getContext(argv: string[]): Promise<Context> {
"--V": "--version",
"--no-color": Boolean,
"--no-motion": Boolean,
"--overwrite": Boolean,
},
{ argv, permissive: true }
);
Expand All @@ -110,6 +114,7 @@ async function getContext(argv: string[]): Promise<Context> {
"--no-motion": noMotion,
"--yes": yes,
"--version": versionRequested,
"--overwrite": overwrite,
} = flags;

let cwd = flags["_"][0] as string;
Expand Down Expand Up @@ -137,7 +142,12 @@ async function getContext(argv: string[]): Promise<Context> {
}

let context: Context = {
tempDir: path.join(
await fs.promises.realpath(os.tmpdir()),
`create-remix--${Math.random().toString(36).substr(2, 8)}`
),
cwd,
overwrite,
interactive,
debug,
git: git ?? (noGit ? false : yes),
Expand Down Expand Up @@ -165,6 +175,7 @@ async function getContext(argv: string[]): Promise<Context> {
}

interface Context {
tempDir: string;
cwd: string;
interactive: boolean;
debug: boolean;
Expand All @@ -184,6 +195,7 @@ interface Context {
template?: string;
token?: string;
versionRequested?: boolean;
overwrite?: boolean;
}

async function introStep(ctx: Context) {
Expand All @@ -204,51 +216,28 @@ async function introStep(ctx: Context) {
}

async function projectNameStep(ctx: Context) {
let cwdIsEmpty = ctx.cwd && isEmpty(ctx.cwd);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We no longer care if it's empty


// valid cwd is required if shell isn't interactive
if (!ctx.interactive) {
if (!ctx.cwd) {
error("Oh no!", "No project directory provided");
throw new Error("No project directory provided");
}

if (!cwdIsEmpty) {
error(
"Oh no!",
`Project directory "${color.reset(ctx.cwd)}" is not empty`
);
throw new Error("Project directory is not empty");
}
if (!ctx.interactive && !ctx.cwd) {
error("Oh no!", "No project directory provided");
throw new Error("No project directory provided");
}

if (ctx.cwd) {
await sleep(100);

if (cwdIsEmpty) {
info("Directory:", [
"Using ",
color.reset(ctx.cwd),
" as project directory",
]);
} else {
info("Hmm...", [color.reset(`"${ctx.cwd}"`), " is not empty!"]);
}
info("Directory:", [
"Using ",
color.reset(ctx.cwd),
" as project directory",
]);
}

if (!ctx.cwd || !cwdIsEmpty) {
if (!ctx.cwd) {
let { name } = await ctx.prompt({
name: "name",
type: "text",
label: title("dir"),
message: "Where should we create your new project?",
initial: "./my-remix-app",
validate(value: string) {
if (!isEmpty(value)) {
return `Directory is not empty!`;
}
return true;
},
});
ctx.cwd = name!;
ctx.projectName = toValidProjectName(name!);
Expand All @@ -266,7 +255,7 @@ async function projectNameStep(ctx: Context) {
ctx.projectName = toValidProjectName(name);
}

async function templateStep(ctx: Context) {
async function copyTemplateToTempDirStep(ctx: Context) {
if (ctx.template) {
log("");
info("Template", ["Using ", color.reset(ctx.template), "..."]);
Expand All @@ -285,41 +274,27 @@ async function templateStep(ctx: Context) {
start: "Template copying...",
end: "Template copied",
while: async () => {
let destPath = path.resolve(process.cwd(), ctx.cwd);
await ensureDirectory(destPath);
await copyTemplate(template, destPath, {
await ensureDirectory(ctx.tempDir);
if (ctx.debug) {
info(`Extracting template to temp directory: ${ctx.tempDir}`);
}

// TODO: Optimization - if template is just a local directory (not a
// local tarball etc.), just use that as ctx.tempDir for the rest of
// the pipeline and avoid copying it to a temp directory

await copyTemplate(template, ctx.tempDir, {
debug: ctx.debug,
token: ctx.token,
async onError(err) {
let cwd = process.cwd();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to do this cleanup anymore - it's extracting into a temp dir which the OS should cleanup automatically.

let removing = (async () => {
if (cwd !== destPath && !pathContains(cwd, destPath)) {
try {
await rm(destPath);
} catch (_) {
error("Oh no!", ["Failed to remove ", destPath]);
}
}
})();
if (ctx.debug) {
try {
await removing;
} catch (_) {}
throw err;
}

await Promise.all([
error(
"Oh no!",
err instanceof CopyTemplateError
? err.message
: "Something went wrong. Run `create-remix --debug` to see more info.\n\n" +
"Open an issue to report the problem at " +
"https://github.com/remix-run/remix/issues/new"
),
removing,
]);

error(
"Oh no!",
err instanceof CopyTemplateError
? err.message
: "Something went wrong. Run `create-remix --debug` to see more info.\n\n" +
"Open an issue to report the problem at " +
"https://github.com/remix-run/remix/issues/new"
);
throw err;
},
async log(message) {
Expand All @@ -329,12 +304,44 @@ async function templateStep(ctx: Context) {
}
},
});

await updatePackageJSON(ctx);
},
ctx,
});
}

async function copyTempDirToAppDirStep(ctx: Context) {
await ensureDirectory(ctx.cwd);

let files1 = await getDirectoryFilesRecursive(ctx.tempDir);
let files2 = await getDirectoryFilesRecursive(ctx.cwd);
let collisions = files1.filter((f) => files2.includes(f));

if (collisions.length > 0 && !ctx.overwrite) {
if (ctx.debug) {
info(`Colliding files:${["", ...collisions].join("\n ")}`);
}

let files = `${collisions.slice(0, 3).join(", ")}${
collisions.length > 3 ? ` and ${collisions.length - 3} more...` : ""
}`;
let { overwrite } = await ctx.prompt({
name: "overwrite",
type: "confirm",
label: title("overwrite"),
message:
`Your app directory already contains files that will be overwritten\n` +
` by this template. Do you wish to continue?\n` +
` Colliding files: ${files}\n` +
` (You can skip this message with the --overwrite CLI flag)`,
initial: false,
});
if (!overwrite) {
throw new Error("Exiting to avoid overwriting files");
}
}

await fse.copy(ctx.tempDir, ctx.cwd);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the population of the app directory now

await updatePackageJSON(ctx);
ctx.initScriptPath = await getInitScriptPath(ctx.cwd);
}

Expand Down Expand Up @@ -590,23 +597,6 @@ async function doneStep(ctx: Context) {
await sleep(200);
}

function isEmpty(dirPath: string) {
if (!fs.existsSync(dirPath)) {
return true;
}

// Some existing files can be safely ignored when checking if
// a directory is a valid project directory.
let VALID_PROJECT_DIRECTORY_SAFE_LIST = [".DS_Store", "Thumbs.db"];

let conflicts = fs.readdirSync(dirPath).filter((content) => {
return !VALID_PROJECT_DIRECTORY_SAFE_LIST.some((safeContent) => {
return content === safeContent;
});
});
return conflicts.length === 0;
}

type PackageManager = "npm" | "yarn" | "pnpm";

const packageManagerExecScript: Record<PackageManager, string> = {
Expand Down
2 changes: 2 additions & 0 deletions packages/create-remix/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"log-update": "^5.0.1",
"node-fetch": "^2.6.9",
"proxy-agent": "^6.3.0",
"recursive-readdir": "^2.2.3",
"rimraf": "^4.1.2",
"semver": "^7.3.7",
"sisteransi": "^1.0.5",
Expand All @@ -34,6 +35,7 @@
"devDependencies": {
"@types/gunzip-maybe": "^1.4.0",
"@types/node-fetch": "^2.5.7",
"@types/recursive-readdir": "^2.2.1",
"@types/tar-fs": "^2.0.1",
"esbuild": "0.17.6",
"esbuild-register": "^3.3.2",
Expand Down
25 changes: 25 additions & 0 deletions packages/create-remix/utils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import path from "node:path";
import process from "node:process";
import os from "node:os";
import fs from "node:fs";
import { type Key as ActionKey } from "node:readline";
import { erase, cursor } from "sisteransi";
import chalk from "chalk";
import recursiveReaddir from "recursive-readdir";

// https://no-color.org/
const SUPPORTS_COLOR = chalk.supportsColor && !process.env.NO_COLOR;
Expand Down Expand Up @@ -266,3 +268,26 @@ export function action(key: ActionKey, isSelect: boolean) {

return false;
}

export async function getDirectoryFilesRecursive(d: string) {
// Ignore comparing within these directories - but detect a collision
// at the directory level and count it. These get prepended to strippedFiles
// in reverse order below
let ignoreDirs = ["node_modules", ".git"];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't check file-by-file inside .git/node_modules but we treat them as collisions regardless. There's a part of me that wants to add a hard stop on ever allowing an overwrite of .git/ though...Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree with this sentiment, overwriting .git feels pretty much guaranteed to be a mistake.

Rather than being an error, maybe we should just never copy the .git directory if it exists in a template? Especially since it's not possible to have one when the template is a GitHub repo, which is the primary mechanism for sharing a template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooh, yeah I like that even better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let dirPrefix = new RegExp(`^${d}${path.sep}+`);
let files = await recursiveReaddir(d, [
(file) => {
let strippedFile = file.replace(dirPrefix, "");
let parts = strippedFile.split(path.sep);
return parts.length > 1 && ignoreDirs.includes(parts[0]);
},
]);
let strippedFiles = files.map((f) => f.replace(dirPrefix, ""));
ignoreDirs.forEach((dir) => {
let dirPath = path.join(d, dir);
if (fs.existsSync(dirPath) && fs.statSync(dirPath).isDirectory()) {
strippedFiles.unshift(dirPath.replace(dirPrefix, ""));
}
});
return strippedFiles;
}
14 changes: 14 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3321,6 +3321,13 @@
"@types/scheduler" "*"
csstype "^3.0.2"

"@types/recursive-readdir@^2.2.1":
version "2.2.1"
resolved "https://registry.npmjs.org/@types/recursive-readdir/-/recursive-readdir-2.2.1.tgz#330f5ec0b73e8aeaf267a6e056884e393f3543a3"
integrity sha512-Xd+Ptc4/F2ueInqy5yK2FI5FxtwwbX2+VZpcg+9oYsFJVen8qQKGapCr+Bi5wQtHU1cTXT8s+07lo/nKPgu8Gg==
dependencies:
"@types/node" "*"

"@types/[email protected]":
version "1.17.1"
resolved "https://registry.npmjs.org/@types/resolve/-/resolve-1.17.1.tgz"
Expand Down Expand Up @@ -11634,6 +11641,13 @@ rechoir@^0.6.2:
dependencies:
resolve "^1.1.6"

recursive-readdir@^2.2.3:
version "2.2.3"
resolved "https://registry.npmjs.org/recursive-readdir/-/recursive-readdir-2.2.3.tgz#e726f328c0d69153bcabd5c322d3195252379372"
integrity sha512-8HrF5ZsXk5FAH9dgsx3BlUer73nIhuj+9OrQwEbLTPOBzGkL1lsFCR01am+v+0m2Cmbs1nP12hLDl5FA7EszKA==
dependencies:
minimatch "^3.0.5"

redent@^3.0.0:
version "3.0.0"
resolved "https://registry.npmjs.org/redent/-/redent-3.0.0.tgz"
Expand Down