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

fetch() doesn't return correct Response type #4395

Closed
christian-reichart opened this issue Oct 20, 2022 · 10 comments
Closed

fetch() doesn't return correct Response type #4395

christian-reichart opened this issue Oct 20, 2022 · 10 comments
Assignees
Labels
bug:unverified feat:fetch Issues related to @remix-run/web-fetch

Comments

@christian-reichart
Copy link

christian-reichart commented Oct 20, 2022

What version of Remix are you using?

1.7.2

Steps to Reproduce

Inside the remix project try

const a = await fetch('https://google.com')
console.log('a instanceof Response: ', a instanceof Response) // -> false

This probably has to do with this commit:
25b4c83#diff-87ec1c3c130fe3481b9f1c542b1d8a53548882d33c73c68b894fd442d606afc0

A current workaround I found thanks to @milamer is to include

import { Response } from '@remix-run/web-fetch'

in the file.

Expected Behavior

instanceof Response should be true

Actual Behavior

instanceof Response is false

@machour
Copy link
Collaborator

machour commented Oct 31, 2022

Hi @christian-reichart, where are you importing fetch from?

@machour machour added the needs-response We need a response from the original author about this issue/PR label Oct 31, 2022
@christian-reichart
Copy link
Author

Hi @machour , I don't import it specifically, so it's the global fetch.

@machour
Copy link
Collaborator

machour commented Oct 31, 2022

Any reason why you're not using remix's fetch?

import { fetch } from "@remix-run/node";

@christian-reichart
Copy link
Author

Thanks for your response @machour ! Correct me if I'm wrong but I was under the impression that Remix is supposed to let you use fetch and Response without any import. https://remix.run/docs/en/v1/other-api/fetch Is the documentation outdated?

@machour
Copy link
Collaborator

machour commented Nov 1, 2022

Now I'm the one confused 😅 I'll circle back with the team on this point and let you know what's the right approach, and will update the docs if necessary.

In the meantime, does using fetch from @remix-run/node fix your original issue?

You might also be interested in reading the "Moving closer to The Platform™" part from the 1.5.0 release notes : https://github.com/remix-run/remix/releases/tag/v1.5.0

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Nov 3, 2022
@acoreyj
Copy link

acoreyj commented Jan 13, 2023

@machour I have run into this issue with reimix-run/node and external libraries that use globals and instanceof Response.

I have been able to work around but latest is in a nested dependency using oauth4webapi

Any thoughts from the team yet on how to fix the inconsistent globals?

In the router they worked around it as well for more context

remix-run/react-router#9690

Although if it is an issue with esm vs cjs not sure what the solution could be

acoreyj pushed a commit to acoreyj/remix that referenced this issue Jan 13, 2023
acoreyj pushed a commit to acoreyj/remix that referenced this issue Jan 13, 2023
@acoreyj
Copy link

acoreyj commented Jan 14, 2023

Found fix and made a merge request.

Here is patch that can be used with patch-package https://gist.github.com/acoreyj/37eff1fdc459dbea31515d5d40be7b35

@jacob-ebey
Copy link
Member

Should be closed by #7109

@jacob-ebey jacob-ebey moved this from In progress to Merged in v2 Aug 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-a179aa7-20230809 which involves this issue. 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
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-b1149bb-20230810 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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