Repository: remix-run/remix
Pull request #3087 (2022-05-04 02:24:58)

chore(remix-dev): only check if dir is empty, not if it exists
<!--

👋 Hey, thanks for your interest in contributing to Remix!

If this is a simple docs change, go ahead and delete all this.

**Please ask first before starting work on any significant new features.**

It's never a fun experience to have your pull request declined after investing a
lot of time and effort into a new feature. To avoid this from happening, we
request that contributors create a
[Feature Request discussion](https://github.com/remix-run/remix/discussions/new?category=ideas)
to first discuss any significant new features.

https://github.com/remix-run/remix/blob/main/CONTRIBUTING.md

Please fill in or delete each item below:

-->

Closes: #2828

- [ ] Docs
- [x] Tests

Testing Strategy:

This test covers this code: https://github.com/remix-run/remix/pull/3087/files#diff-44fd7b47d36d99648d628196603ec193162f8cc64062cde2d76bbe36f89c6356R624

I also ran the build `yarn build` and ran the following:
success cases:
```shell
cd ~/Desktop && \
mkdir test && \
cd test && \
node ~/<path-to-remix>/build/node_modules/create-remix/cli.js .
```

and also
```shell
mkdir ~/Desktop/test && \
node ./build/node_modules/create-remix/cli.js ~/Desktop/test
```

failing cases:
```shell
cd ~/Desktop && \
mkdir test && \
cd test && \
touch .some-file && \
node ~/<path-to-remix>/build/node_modules/create-remix/cli.js .
```

and also
```shell
mkdir ~/Desktop/test && \
touch ~/Desktop/test/.some-file && \
node ./build/node_modules/create-remix/cli.js ~/Desktop/test
```



<!--
Please explain how you tested this. For example:

> This test covers this code: <link to test>

Or

> I opened up my windows machine and ran this script:
>
> ```
> npx create-remix@0.0.0-experimental-7e420ee3 --template remix my-test
> cd my-test
> npm run dev
> ```
-->



Patch:
diff --git a/packages/remix-dev/cli/create.ts b/packages/remix-dev/cli/create.ts
index 0264e41e3..c75a8f276 100644
--- a/packages/remix-dev/cli/create.ts
+++ b/packages/remix-dev/cli/create.ts
@@ -38,17 +38,6 @@ export async function createApp({
   githubToken = process.env.GITHUB_TOKEN,
   debug,
 }: CreateAppArgs) {
-  // Create the app directory
-  let relativeProjectDir = path.relative(process.cwd(), projectDir);
-  let projectDirIsCurrentDir = relativeProjectDir === "";
-  if (!projectDirIsCurrentDir) {
-    if (fse.existsSync(projectDir)) {
-      throw new Error(
-        `️🚨 Oops, "${relativeProjectDir}" already exists. Please try again with a different directory.`
-      );
-    }
-  }
-
   /**
    * Grab the template
    * First we'll need to determine if the template we got is
@@ -443,14 +432,9 @@ export async function validateNewProjectPath(input: string): Promise<void> {
   ) {
     if ((await fse.readdir(projectDir)).length > 0) {
       throw Error(
-        "🚨 The current directory must be empty to create a new project. Please " +
+        "🚨 The project directory must be empty to create a new project. Please " +
           "clear the contents of the directory or choose a different path."
       );
-    } else {
-      throw Error(
-        "🚨 The directory provided already exists. Please try again with a " +
-          "different directory."
-      );
     }
   }
 }
diff --git a/packages/remix-eslint-config/rules/core.js b/packages/remix-eslint-config/rules/core.js
index 7f328dd25..1fe183487 100644
--- a/packages/remix-eslint-config/rules/core.js
+++ b/packages/remix-eslint-config/rules/core.js
@@ -1,44 +1,45 @@
-const {
-  defaultAdapterExports,
-  defaultRuntimeExports,
-  architectSpecificExports,
-  cloudflareSpecificExports,
-  cloudflarePagesSpecificExports,
-  cloudflareWorkersSpecificExports,
-  nodeSpecificExports,
-  reactSpecificExports,
-} = require("./packageExports");
+// const {
+//   defaultAdapterExports,
+//   defaultRuntimeExports,
+//   architectSpecificExports,
+//   cloudflareSpecificExports,
+//   cloudflarePagesSpecificExports,
+//   cloudflareWorkersSpecificExports,
+//   nodeSpecificExports,
+//   reactSpecificExports,
+// } = require("./packageExports");
 
 // const OFF = 0;
 const WARN = 1;
 const ERROR = 2;
 
-const getReplaceRemixImportsMessage = (packageName) =>
-  `All \`remix\` exports are considered deprecated as of v1.3.3. Please use \`@remix-run/${packageName}\` instead. You can run \`remix migrate --migration replace-remix-imports\` to automatically migrate your code.`;
-const replaceRemixImportsOptions = [
-  {
-    packageExports: defaultAdapterExports,
-    packageName:
-      "{architect|cloudflare-pages|cloudflare-workers|express|netlify|vercel}",
-  },
-  { packageExports: defaultRuntimeExports, packageName: "{cloudflare|node}" },
-  { packageExports: architectSpecificExports, packageName: "architect" },
-  { packageExports: cloudflareSpecificExports, packageName: "cloudflare" },
-  {
-    packageExports: cloudflarePagesSpecificExports,
-    packageName: "cloudflare-pages",
-  },
-  {
-    packageExports: cloudflareWorkersSpecificExports,
-    packageName: "cloudflare-workers",
-  },
-  { packageExports: nodeSpecificExports, packageName: "node" },
-  { packageExports: reactSpecificExports, packageName: "react" },
-].map(({ packageExports, packageName }) => ({
-  importNames: [...packageExports.value, ...packageExports.type],
-  message: getReplaceRemixImportsMessage(packageName),
-  name: "remix",
-}));
+// const getReplaceRemixImportsMessage = (packageName) =>
+//   `All \`remix\` exports are considered deprecated as of v1.3.3. Please use \`@remix-run/${packageName}\` instead. You can run \`remix migrate --migration replace-remix-imports\` to automatically migrate your code.`;
+
+//   const replaceRemixImportsOptions = [
+//   {
+//     packageExports: defaultAdapterExports,
+//     packageName:
+//       "{architect|cloudflare-pages|cloudflare-workers|express|netlify|vercel}",
+//   },
+//   { packageExports: defaultRuntimeExports, packageName: "{cloudflare|node}" },
+//   { packageExports: architectSpecificExports, packageName: "architect" },
+//   { packageExports: cloudflareSpecificExports, packageName: "cloudflare" },
+//   {
+//     packageExports: cloudflarePagesSpecificExports,
+//     packageName: "cloudflare-pages",
+//   },
+//   {
+//     packageExports: cloudflareWorkersSpecificExports,
+//     packageName: "cloudflare-workers",
+//   },
+//   { packageExports: nodeSpecificExports, packageName: "node" },
+//   { packageExports: reactSpecificExports, packageName: "react" },
+// ].map(({ packageExports, packageName }) => ({
+//   importNames: [...packageExports.value, ...packageExports.type],
+//   message: getReplaceRemixImportsMessage(packageName),
+//   name: "remix",
+// }));
 
 module.exports = {
   "array-callback-return": WARN,
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index a22655260..9cde1cb21 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -911,54 +911,10 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
     let formMethod: FormMethod =
       method.toLowerCase() === "get" ? "get" : "post";
     let formAction = useFormAction(action);
-    let formRef = React.useRef<HTMLFormElement>();
-    let ref = useComposedRefs(forwardedRef, formRef);
-
-    // When calling `submit` on the form element itself, we don't get data from
-    // the button that submitted the event. For example:
-    //
-    //   <Form>
-    //     <button name="something" value="whatever">Submit</button>
-    //   </Form>
-    //
-    // formData.get("something") should be "whatever", but we don't get that
-    // unless we call submit on the clicked button itself.
-    //
-    // To figure out which button triggered the submit, we'll attach a click
-    // event listener to the form. The click event is always triggered before
-    // the submit event (even when submitting via keyboard when focused on
-    // another form field, yeeeeet) so we should have access to that button's
-    // data for use in the submit handler.
-    let clickedButtonRef = React.useRef<any>();
-
-    React.useEffect(() => {
-      let form = formRef.current;
-      if (!form) return;
-
-      function handleClick(event: MouseEvent) {
-        if (!(event.target instanceof Element)) return;
-        let submitButton = event.target.closest<
-          HTMLButtonElement | HTMLInputElement
-        >("button,input[type=submit]");
-
-        if (
-          submitButton &&
-          submitButton.form === form &&
-          submitButton.type === "submit"
-        ) {
-          clickedButtonRef.current = submitButton;
-        }
-      }
-
-      window.addEventListener("click", handleClick);
-      return () => {
-        window.removeEventListener("click", handleClick);
-      };
-    }, []);
 
     return (
       <form
-        ref={ref}
+        ref={forwardedRef}
         method={formMethod}
         action={formAction}
         encType={encType}
@@ -970,11 +926,10 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
                 if (event.defaultPrevented) return;
                 event.preventDefault();
 
-                submit(clickedButtonRef.current || event.currentTarget, {
-                  method,
-                  replace,
-                });
-                clickedButtonRef.current = null;
+                let submitter = (event as unknown as HTMLSubmitEvent)
+                  .nativeEvent.submitter as HTMLFormSubmitter | null;
+
+                submit(submitter || event.currentTarget, { method, replace });
               }
         }
         {...props}
@@ -985,6 +940,14 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
 FormImpl.displayName = "FormImpl";
 export { FormImpl };
 
+type HTMLSubmitEvent = React.BaseSyntheticEvent<
+  SubmitEvent,
+  Event,
+  HTMLFormElement
+>;
+
+type HTMLFormSubmitter = HTMLButtonElement | HTMLInputElement;
+
 /**
  * Resolves a `<form action>` path relative to the current route.
  *
@@ -1422,7 +1385,7 @@ export const LiveReload =
                   let socketPath = protocol + "//" + host + ":" + ${String(
                     port
                   )} + "/socket";
-        
+
                   let ws = new WebSocket(socketPath);
                   ws.onmessage = (message) => {
                     let event = JSON.parse(message.data);
@@ -1444,21 +1407,3 @@ export const LiveReload =
           />
         );
       };
-
-function useComposedRefs<RefValueType = any>(
-  ...refs: Array<React.Ref<RefValueType> | null | undefined>
-): React.RefCallback<RefValueType> {
-  return React.useCallback((node) => {
-    for (let ref of refs) {
-      if (ref == null) continue;
-      if (typeof ref === "function") {
-        ref(node);
-      } else {
-        try {
-          (ref as React.MutableRefObject<RefValueType>).current = node!;
-        } catch (_) {}
-      }
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, refs);
-}


Test patch:
diff --git a/packages/remix-dev/__tests__/create-test.ts b/packages/remix-dev/__tests__/create-test.ts
index 9e2dda96b..571e29a1b 100644
--- a/packages/remix-dev/__tests__/create-test.ts
+++ b/packages/remix-dev/__tests__/create-test.ts
@@ -402,7 +402,6 @@ describe("the create command", () => {
     expect(fse.existsSync(path.join(projectDir, "test.txt"))).toBeTruthy();
     // if you run `remix init` keep around the remix.init directory for future use
     expect(fse.existsSync(path.join(projectDir, "remix.init"))).toBeTruthy();
-    // deps can take a bit to install
   });
 
   it("throws an error when invalid remix.init script when automatically ran", async () => {
@@ -422,7 +421,6 @@ describe("the create command", () => {
     expect(fse.existsSync(path.join(projectDir, "app/root.tsx"))).toBeTruthy();
     // we should keep remix.init around if the init script fails
     expect(fse.existsSync(path.join(projectDir, "remix.init"))).toBeTruthy();
-    // deps can take a bit to install
   });
 
   it("throws an error when invalid remix.init script when manually ran", async () => {
@@ -449,7 +447,6 @@ describe("the create command", () => {
     expect(fse.existsSync(path.join(projectDir, "app/root.tsx"))).toBeTruthy();
     // we should keep remix.init around if the init script fails
     expect(fse.existsSync(path.join(projectDir, "remix.init"))).toBeTruthy();
-    // deps can take a bit to install
   });
 
   it("recognizes when Yarn was used to run the command", async () => {
@@ -623,6 +620,81 @@ describe("the create command", () => {
         `[Error: 🚨 The template file could not be verified. The server returned a response with a 400 status. Please double check the URL and try again.]`
       );
     });
+
+    it("allows creating an app in a dir if it's empty", async () => {
+      let projectDir = await getProjectDir("other-empty-dir");
+      await run([
+        "create",
+        projectDir,
+        "--template",
+        "grunge-stack",
+        "--no-install",
+        "--typescript",
+      ]);
+
+      expect(
+        fse.existsSync(path.join(projectDir, "package.json"))
+      ).toBeTruthy();
+    });
+
+    it("doesn't allow creating an app in a dir if it's not empty", async () => {
+      let projectDir = await getProjectDir("not-empty-dir");
+      fse.mkdirSync(projectDir);
+      fse.createFileSync(path.join(projectDir, "some-file.txt"));
+      await expect(() =>
+        run([
+          "create",
+          projectDir,
+          "--template",
+          "grunge-stack",
+          "--no-install",
+          "--typescript",
+        ])
+      ).rejects.toMatchInlineSnapshot(
+        `[Error: 🚨 The project directory must be empty to create a new project. Please clear the contents of the directory or choose a different path.]`
+      );
+    });
+
+    it("allows creating an app in the current dir if it's empty", async () => {
+      let projectDir = await getProjectDir("empty-dir");
+      let cwd = process.cwd();
+      fse.mkdirSync(projectDir);
+      process.chdir(projectDir);
+      await run([
+        "create",
+        ".",
+        "--template",
+        "grunge-stack",
+        "--no-install",
+        "--typescript",
+      ]);
+      process.chdir(cwd);
+
+      expect(
+        fse.existsSync(path.join(projectDir, "package.json"))
+      ).toBeTruthy();
+    });
+
+    it("doesn't allow creating an app in the current dir if it's not empty", async () => {
+      let projectDir = await getProjectDir("not-empty-dir");
+      let cwd = process.cwd();
+      fse.mkdirSync(projectDir);
+      fse.createFileSync(path.join(projectDir, "some-file.txt"));
+      process.chdir(projectDir);
+      await expect(() =>
+        run([
+          "create",
+          ".",
+          "--template",
+          "grunge-stack",
+          "--no-install",
+          "--typescript",
+        ])
+      ).rejects.toMatchInlineSnapshot(
+        `[Error: 🚨 The project directory must be empty to create a new project. Please clear the contents of the directory or choose a different path.]`
+      );
+      process.chdir(cwd);
+    });
   });
 });