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

[vite] Form data not being parsed in actions with custom entry.server and entry.client #8050

Closed
AlemTuzlak opened this issue Nov 18, 2023 · 6 comments

Comments

@AlemTuzlak
Copy link
Contributor

AlemTuzlak commented Nov 18, 2023

Reproduction

https://github.com/AlemTuzlak/formdata-repro

System Info

System:
    OS: Windows 11 10.0.22621
    CPU: (24) x64 12th Gen Intel(R) Core(TM) i9-12900K
    Memory: 7.83 GB / 31.71 GB
  Binaries:
    Node: 18.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.4.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.6.9 - ~\AppData\Roaming\npm\pnpm.CMD
  Browsers:
    Chrome: 119.0.6045.160
    Edge: Chromium (119.0.2151.72)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @remix-run/dev: ^2.3.0 => 2.3.0
    @remix-run/eslint-config: ^2.3.0 => 2.3.0
    @remix-run/node: ^2.3.0 => 2.3.0
    @remix-run/react: ^2.3.0 => 2.3.0
    @remix-run/serve: ^2.3.0 => 2.3.0
    vite: ^5.0.0 => 5.0.0

Used Package Manager

npm

Expected Behavior

submit data is parsed by request.formData and you get it correctly

Actual Behavior

The data is sent over the wire but when you do request.formData() it's empty

Removing entry client and server makes it work again

there is no formData even though it's sent

@naghtigal
Copy link

I'm experiencing the same problem when using the Vite plugin. Once I use entry.server - formData in action is empty. If I delete entry.server - formData is passed properly.

@pcattori pcattori added the vite label Nov 18, 2023
@hi-ogawa
Copy link
Contributor

hi-ogawa commented Nov 18, 2023

I noticed that your reproduction is using my hacky workaround I wrote in #7819 (comment).

https://github.com/AlemTuzlak/formdata-repro/blob/aff243a645ca9f6243393c853ebee12b0302c164/app/entry.server.tsx#L23

  (globalThis as any).Request = request.constructor; //NOTE: for now this is the only solution: https://github.com/remix-run/remix/issues/7819

I think It's likely that this would break certain Request related functionality in other places. What I intended there was only a demo to show there are two distinct Request classes (sorry if my comment was confusing...)

Just in case, I verified formData works after remix reveal with the original template (no i18n or Request hack) https://stackblitz.com/edit/remix-run-remix-z7nswj?file=app%2Froutes%2F_index.tsx

@hi-ogawa
Copy link
Contributor

Btw, thanks for the reproduction!
Until remix vite sorts out Request polyfill issue, I think it might be better to put workaround on ... instanceof Response side.
For example, this patch hi-ogawa/remix-8050-formdata-repro@d9117bd could work for remix-i18next.
I put this reproduction on stackblitz as well https://stackblitz.com/edit/hi-ogawa-remix-8050-formdata-repro-mm6agz?file=patches%2Fremix-i18next%2B5.4.0.patch

reveal screenshot

image

(But, maybe I should stop sharing workaround after workaround since that might just bring more confusing issues elsewhere...)

@AlemTuzlak
Copy link
Contributor Author

@hi-ogawa You're right about the workaround, we were using that but that is obviously a workaround, what you might want to consider is doing something like this:

config(config){
  config.define = { 
    globalThis: {
      Request: yourRequestPoly.constructor
    }
  }
}

in the remix plugin, this would just extend the global definitions but I'm not sure if this in itself would be a workaround or there is a better solution, just my 2 cents, I'm assuming the PR you made is the actual fix. Thank you for this in any case!

@markdalgleish markdalgleish self-assigned this Nov 23, 2023
@markdalgleish
Copy link
Member

Thanks for the repro! That really helps a lot.

As @hi-ogawa said, it looks like your issue is actually this one: #7819 — and that your issues with form data are due to the fact that the workaround is suppressing the underlying bug rather than fixing it. This issue should be fixed once #8062 is released and the workaround is removed.

@markdalgleish
Copy link
Member

markdalgleish commented Nov 23, 2023

@AlemTuzlak Just to confirm, I was able to fix your form data issue by copying the updated Node adapter from #8062 into your repro's node_modules and removing the workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants