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

brophdawg11 · 2023-08-04T12:55:56Z

We first implemented this as an --allow-non-empty flag.

We pivoted and removed the empty directory requirement and moved to a more granular file-by-file check. You are now free to write into a non-empty directory. If no collisions are detected between the template and your directory - then it'll run normally with no additional user input.

If collisions are detected, we prompt for confirmation to overwrite:

Users can skip the confirmation prompt with --overwrite.

Closes #6676

changeset-bot · 2023-08-04T12:56:01Z

🦋 Changeset detected

Latest commit: a310c86

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
create-remix	Minor
remix	Minor
@remix-run/architect	Minor
@remix-run/cloudflare	Minor
@remix-run/cloudflare-pages	Minor
@remix-run/cloudflare-workers	Minor
@remix-run/css-bundle	Minor
@remix-run/deno	Minor
@remix-run/dev	Minor
@remix-run/eslint-config	Minor
@remix-run/express	Minor
@remix-run/node	Minor
@remix-run/react	Minor
@remix-run/serve	Minor
@remix-run/server-runtime	Minor
@remix-run/testing	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/create-remix/__tests__/create-remix-test.ts

markdalgleish

I think --allow-non-empty is okay given your point that it doesn't necessarily overwrite anything.

Should we include a note about the --allow-non-empty flag in the non-empty directory error message? Otherwise I feel like devs won't know this flag exists in the scenario from the linked issue.

Looks good otherwise, so I'm happy to approve and leave it with you :)

brophdawg11 · 2023-08-08T17:25:12Z

Chatted with @mjackson and we're going to make this a bit more granular. We shouldn't care if the directory is empty, instead we should only care if we're actually going to overwrite stuff. I'm going to update this to:

No longer require an empty directory
Check for collisions with existing files, and if found, prompt: "Are you sure you wish to overwrite?"
Change the flag to --overwrite to disable the prompt above

This reverts commit 51525f2.

Dismissing review due to alternate approach

brophdawg11 · 2023-08-17T20:47:45Z

packages/create-remix/index.ts

+    copyTemplateToTempDirStep,
+    copyTempDirToAppDirStep,


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.

brophdawg11 · 2023-08-17T20:48:06Z

packages/create-remix/index.ts

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

 async function projectNameStep(ctx: Context) {
-  let cwdIsEmpty = ctx.cwd && isEmpty(ctx.cwd);


We no longer care if it's empty

brophdawg11 · 2023-08-17T20:48:58Z

packages/create-remix/index.ts

        debug: ctx.debug,
        token: ctx.token,
        async onError(err) {
-          let cwd = process.cwd();


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

brophdawg11 · 2023-08-17T20:49:38Z

packages/create-remix/index.ts


+  await fse.copy(ctx.tempDir, ctx.cwd);


This is the population of the app directory now

brophdawg11 · 2023-08-17T20:51:02Z

packages/create-remix/utils.ts

+  // 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"];


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?

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.

ooh, yeah I like that even better

.changeset/create-remix-override.md

Co-authored-by: Mark Dalgleish <[email protected]>

This reverts commit 8e0180a.

brophdawg11 · 2023-08-21T14:47:14Z

packages/create-remix/__tests__/create-remix-test.ts

+    expect(fse.existsSync(path.join(emptyDir, "app/root.tsx"))).toBeTruthy();
+  });
+
+  it("does not copy .git nor node_modules directories if they exist in the template", async () => {


@markdalgleish I decided to treat both .git and node_modules the same and not copy either of them from the template directory. We probably shouldn't promote bloated templates with committed node_modules that we have to download.

brophdawg11 · 2023-08-21T14:48:33Z

packages/create-remix/index.ts

+      if (result?.localTemplateDirectory) {
+        ctx.tempDir = path.resolve(result.localTemplateDirectory);
+      }


Added the optimization that if the template is a local directory on disk we just copy from there instead of "extracting" it to a temp dir.

github-actions · 2023-08-23T07:06:36Z

🤖 Hello there,

We just published version v0.0.0-nightly-49e8da1-20230823 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions · 2023-08-28T22:21:03Z

🤖 Hello there,

We just published version 2.0.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

remix-cla-bot bot added the CLA Signed label Aug 4, 2023

brophdawg11 requested a review from markdalgleish August 4, 2023 12:56

brophdawg11 assigned markdalgleish Aug 4, 2023

markdalgleish reviewed Aug 7, 2023

View reviewed changes

packages/create-remix/__tests__/create-remix-test.ts Outdated Show resolved Hide resolved

markdalgleish previously approved these changes Aug 7, 2023

View reviewed changes

brophdawg11 added the package:create-remix label Aug 7, 2023

MichaelDeBoey changed the title ~~Add allow-non-empty flag to create-remix CLI~~ feat(create-remix): add --allow-non-empty flag Aug 7, 2023

MichaelDeBoey previously approved these changes Aug 7, 2023

View reviewed changes

brophdawg11 assigned brophdawg11 and unassigned markdalgleish Aug 17, 2023

brophdawg11 added 3 commits August 17, 2023 14:12

Add allow-non-empty flag to create-remix CLI

51525f2

Revert "Add allow-non-empty flag to create-remix CLI"

57dbe57

This reverts commit 51525f2.

Add --override prompt/flag to create-remix

7efa734

brophdawg11 force-pushed the brophdawg11/allow-non-empty branch from 3048ccd to 7efa734 Compare August 17, 2023 20:41

Fix docs

62e3b2d

brophdawg11 commented Aug 17, 2023

View reviewed changes

brophdawg11 changed the title ~~feat(create-remix): add --allow-non-empty flag~~ feat(create-remix): add --overwrite flag Aug 17, 2023

MichaelDeBoey approved these changes Aug 17, 2023

View reviewed changes

markdalgleish reviewed Aug 17, 2023

View reviewed changes

.changeset/create-remix-override.md Outdated Show resolved Hide resolved

brophdawg11 and others added 3 commits August 18, 2023 12:14

Clean up some logging and add debug() utility

f54669c

Skip copying .git directory

048ca90

Update .changeset/create-remix-override.md

1cedefa

Co-authored-by: Mark Dalgleish <[email protected]>

brophdawg11 added 6 commits August 18, 2023 12:38

Optimize and remove dup copy of local template directory

6f426a0

Merge branch 'dev' into brophdawg11/allow-non-empty

d972300

Handle non-interactive UI

3cad6f3

add no git init flag to tests

6a9c2c8

fix lint error

1524833

Don't copy node_modules either

ed45485

brophdawg11 force-pushed the brophdawg11/allow-non-empty branch 6 times, most recently from 600a43a to a722933 Compare August 18, 2023 20:37

Add debugging to figure out windows CI tests

8e0180a

brophdawg11 force-pushed the brophdawg11/allow-non-empty branch from a722933 to 8e0180a Compare August 18, 2023 20:51

brophdawg11 added 2 commits August 18, 2023 16:58

Revert "Add debugging to figure out windows CI tests"

09f29e7

This reverts commit 8e0180a.

Fix windows directory stripping

f7dc18d

MichaelDeBoey approved these changes Aug 19, 2023

View reviewed changes

brophdawg11 commented Aug 21, 2023

View reviewed changes

brophdawg11 assigned markdalgleish Aug 21, 2023

Update changeset

a310c86

brophdawg11 merged commit 23544d3 into dev Aug 22, 2023

brophdawg11 deleted the brophdawg11/allow-non-empty branch August 22, 2023 17:33

github-actions bot added the awaiting release This issue has been fixed and will be released soon label Aug 22, 2023

brophdawg11 mentioned this pull request Aug 22, 2023

npx create-remix fails in empty git repo directory #6676

Closed

1 task

github-actions bot mentioned this pull request Aug 28, 2023

chore: Update version for release (pre) #7284

Merged

KarelVerschraegen mentioned this pull request Sep 12, 2023

docs: 10-form-vs-fetcher code blocks formatting #7394

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

brophdawg11 commented Aug 4, 2023 •

edited

Loading

changeset-bot bot commented Aug 4, 2023 •

edited

Loading

markdalgleish left a comment

brophdawg11 commented Aug 8, 2023 •

edited

Loading

brophdawg11 Aug 17, 2023

brophdawg11 Aug 17, 2023

brophdawg11 Aug 17, 2023

brophdawg11 Aug 17, 2023

brophdawg11 Aug 17, 2023

markdalgleish Aug 17, 2023

brophdawg11 Aug 18, 2023

brophdawg11 Aug 18, 2023

brophdawg11 Aug 21, 2023

brophdawg11 Aug 21, 2023

github-actions bot commented Aug 23, 2023

github-actions bot commented Aug 28, 2023

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

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

Conversation

brophdawg11 commented Aug 4, 2023 • edited Loading

changeset-bot bot commented Aug 4, 2023 • edited Loading

🦋 Changeset detected

markdalgleish left a comment

Choose a reason for hiding this comment

brophdawg11 commented Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 23, 2023

github-actions bot commented Aug 28, 2023

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

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

brophdawg11 commented Aug 4, 2023 •

edited

Loading

changeset-bot bot commented Aug 4, 2023 •

edited

Loading

brophdawg11 commented Aug 8, 2023 •

edited

Loading