Repository: remix-run/remix
Pull request #4408 (2022-10-23 00:09:42)

feat(remix-node): add `getFilePath`& `remove` methods to `NodeOnDiskFile`
<!--

👋 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: #3021

- [ ] Docs
- [x] Tests

Testing Strategy:

<!--
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
> ```
-->
One unit test for each new function, added in the remix-node package.

This is basically the same PR as #3034, but it got closed by its author before being merged. I believe this is a useful feature and would very much like to see it added.

changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 25851e679f4492bc6ca0e163480afe6c2c1e7ddc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/julienmonnard/remix/new/new-NodeOnDiskFile-methods?filename=.changeset/gorgeous-tips-type.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fnode%22%3A%20patch%0A---%0A%0Afeat(node)%3A%20add%20new%20methods%20to%20NodeOnDiskFile%20class%0A)


remix-cla-bot[bot]: Hi @julienmonnard,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our **Contributor License Agreement** (CLA). We require this only once.

You may [review the CLA](https://github.com/remix-run/remix/blob/main/CLA.md) and sign it by [adding your name to contributors.yml](https://github.com/julienmonnard/remix/blob/new-NodeOnDiskFile-methods/contributors.yml).

Once the CLA is signed, the `CLA Signed` label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [hello@remix.run](mailto:hello@remix.run?subject=CLA%20for%20open%20source%20contributions).

Thanks!

\- The Remix team
remix-cla-bot[bot]: Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳
cstrnt: Nice copy pasta :D
julienmonnard: Indeed. I'm not trying to hide that fact, it's actually mentioned in the description above.

The main difference with your PR is that this one is still open. I need that feature in my project and would like to see it merged. I was disappointed when you closed yours, and as I cannot reopen a PR, I recreated it.
julienmonnard: Is there something more that should be done for this to be able to make it in one of the next releases?
I guess with the whole react-router/remix stuff happening it was quite tricky, but now that 1.10 wraps it up and has landed, maybe there could be a little slot for this one :)


Patch:
diff --git a/.changeset/afraid-hounds-join.md b/.changeset/afraid-hounds-join.md
new file mode 100644
index 000000000..e20bbfd4d
--- /dev/null
+++ b/.changeset/afraid-hounds-join.md
@@ -0,0 +1,7 @@
+---
+"remix": patch
+"@remix-run/cloudflare": patch
+"@remix-run/cloudflare-workers": patch
+---
+
+Update Cloudflare Workers adapter's `RequestHandler` type to be in line with other adapters
diff --git a/.changeset/flat-cobras-beam.md b/.changeset/flat-cobras-beam.md
new file mode 100644
index 000000000..9f9de96e5
--- /dev/null
+++ b/.changeset/flat-cobras-beam.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/node": minor
+---
+
+Add `remove` and `getFilePath` methods to `NodeOnDiskFile`
diff --git a/contributors.yml b/contributors.yml
index a9ae704ec..17f858c42 100644
--- a/contributors.yml
+++ b/contributors.yml
@@ -224,6 +224,7 @@
 - juhanakristian
 - JulesBlm
 - juliaqiuxy
+- julienmonnard
 - justinnoel
 - justinsalasdev
 - justinwaite
diff --git a/packages/remix-cloudflare-workers/worker.ts b/packages/remix-cloudflare-workers/worker.ts
index e20bb901b..c783121b2 100644
--- a/packages/remix-cloudflare-workers/worker.ts
+++ b/packages/remix-cloudflare-workers/worker.ts
@@ -19,7 +19,7 @@ import { createRequestHandler as createRemixRequestHandler } from "@remix-run/cl
  */
 export type GetLoadContextFunction = (event: FetchEvent) => AppLoadContext;
 
-export type RequestHandler = ReturnType<typeof createRequestHandler>;
+export type RequestHandler = (event: FetchEvent) => Promise<Response>;
 
 /**
  * Returns a request handler for the Cloudflare runtime that serves the
@@ -33,7 +33,7 @@ export function createRequestHandler({
   build: ServerBuild;
   getLoadContext?: GetLoadContextFunction;
   mode?: string;
-}) {
+}): RequestHandler {
   let handleRequest = createRemixRequestHandler(build, mode);
 
   return (event: FetchEvent) => {
diff --git a/packages/remix-node/upload/fileUploadHandler.ts b/packages/remix-node/upload/fileUploadHandler.ts
index ad2836ab9..2f076cd1b 100644
--- a/packages/remix-node/upload/fileUploadHandler.ts
+++ b/packages/remix-node/upload/fileUploadHandler.ts
@@ -1,6 +1,6 @@
 import { randomBytes } from "crypto";
 import { createReadStream, createWriteStream, statSync } from "fs";
-import { rm, mkdir, stat as statAsync } from "fs/promises";
+import { rm, mkdir, stat as statAsync, unlink } from "fs/promises";
 import { tmpdir } from "os";
 import { basename, dirname, extname, resolve as resolvePath } from "path";
 import type { Readable } from "stream";
@@ -231,4 +231,11 @@ export class NodeOnDiskFile implements File {
   public get [Symbol.toStringTag]() {
     return "File";
   }
+
+  remove(): Promise<void> {
+    return unlink(this.filepath);
+  }
+  getFilePath(): string {
+    return this.filepath;
+  }
 }


Test patch:
diff --git a/packages/remix-node/__tests__/fileUploadHandler-test.ts b/packages/remix-node/__tests__/fileUploadHandler-test.ts
index 0d52c6eae..02917450b 100644
--- a/packages/remix-node/__tests__/fileUploadHandler-test.ts
+++ b/packages/remix-node/__tests__/fileUploadHandler-test.ts
@@ -104,4 +104,18 @@ describe("NodeOnDiskFile", () => {
     expect(sliced.size).toBe(slicedRes.length);
     expect(await sliced.text()).toBe(slicedRes);
   });
+
+  it("returns the file path properly", async () => {
+    expect(file.getFilePath()).toEqual(filepath);
+  });
+
+  it("removes the file properly", async () => {
+    let newFilePath = `${filepath}-copy`;
+    fs.copyFileSync(filepath, newFilePath);
+
+    let copiedFile = (file = new NodeOnDiskFile(newFilePath, "text/plain"));
+    expect(fs.existsSync(copiedFile.getFilePath())).toBe(true);
+    await copiedFile.remove();
+    expect(fs.existsSync(copiedFile.getFilePath())).toBe(false);
+  });
 });