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

Expose formData method on incoming request from the interceptor #288

Closed
cliffordfajardo opened this issue Sep 16, 2022 · 7 comments · Fixed by #292
Closed

Expose formData method on incoming request from the interceptor #288

cliffordfajardo opened this issue Sep 16, 2022 · 7 comments · Fixed by #292
Assignees

Comments

@cliffordfajardo
Copy link

cliffordfajardo commented Sep 16, 2022

I was using Remix and had tried to do the following in my server.js file (I'm using Remix express template)

interceptor.on('request', async (request) => {
  if(request.method === "POST"){
      const body = await request.formData() // ❌ error obviously since its not an supported on [InteractiveIsomorphicRequest](https://github.com/mswjs/interceptors/blob/e305d3815851dbdfd674e2b412f2c2e0288fc1de/src/IsomorphicRequest.ts)

In my remix app I was attempting to capture a request from my server and rewrite the request (proxy the request) so that an external API provider thinks its coming from my proxy server (example-proxy.com). Details here I wrote up a stackoverflow question

Aside

Then I remembered that formData, Request, fetch were only introduced a few months ago in in nodejs v18

Would bringing in formData be contingent upon this discussion that Kent wrote or the work @milesrichardson is doing to migrate this library to node v18?

@kettanaito
Copy link
Member

Hey, @cliffordfajardo. Thanks for raising this.

Some request reading methods like .formData() or .blob() are currently not supported by our custom abstraction. This is because we create a custom request representation, so we implement those reading methods by ourselves also. This is not ideal.

With the recent proposal to adhere to Fetch Request and Response, this may be rendered absolute. That is, however, a long-term strategy, and I don't see it happening any time soon. Perhaps an intermediate .formData() API on the IsomorphicRequest would be a good thing to have meanwhile. We can use one of the polyfills in the wild to achieve it, no need to reinvent things on that level.

What do you think?

@kettanaito
Copy link
Member

There's also some discussion on this in mswjs/msw#1327 (comment).

@cliffordfajardo
Copy link
Author

cliffordfajardo commented Sep 16, 2022

Perhaps an intermediate .formData() API on the IsomorphicRequest would be a good thing to have meanwhile. We can use one of the polyfills in the wild to achieve it, no need to reinvent things on that level.
What do you think?

I think this would be a good idea, I'll look around for a polyfill and try it on the IsomorphicRequest request

References

@cliffordfajardo
Copy link
Author

cliffordfajardo commented Sep 16, 2022

@kettanaito - have you seen the @remix-run/web-fetch package from the Remix team?

With your history and context of the codebase do you think IsomorphicRequest.ts could be replaced with request from @remix-run/web-fetch or a slighly modified fork ?

Maybe@remix-run/web-fetch would address what you mentioned here:

Some request reading methods like .formData() or .blob() are currently not supported by our custom abstraction. This is because we create a custom request representation, so we implement those reading methods by ourselves also. This is not ideal.

From @remix-run/web-fetch

EDIT
I just noticed some of the files in the @remix-run/web-fetch repo do use NodeJS APIs in their implementation:

@kettanaito
Copy link
Member

Yes! I think migrating to Remix's polyfill is the best option. We can use it internally for now and in the future expose publicly to MSW consumers.

@kettanaito
Copy link
Member

With #292 I'm migrating this library to use Fetch API Request and Response instances. That will also expose request.formData() on the intercepted requests, alongside many other body reading methods.

This issue is therefore obsolete. It will be closed once that API change lands.

@kettanaito
Copy link
Member

Released: v0.18.0 🎉

This has been released in v0.18.0!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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

Successfully merging a pull request may close this issue.

2 participants