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.

Sign up for GitHub

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

Jump to bottom

🐛 BUG: fetch is unavailable until it isn’t #1106

Closed
Tracked by #1563
jonathantneal opened this issue Aug 13, 2021 · 8 comments
Closed
Tracked by #1563

🐛 BUG: fetch is unavailable until it isn’t #1106

jonathantneal opened this issue Aug 13, 2021 · 8 comments
Labels
help wanted Please help with this issue!

Comments

@jonathantneal
Copy link
Contributor

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

Within the header of an .astro file, external .js files cannot usefetch().

Example

/src/pages/index.astro:

---
import { fetched_metadata } from '../fetched.js'

const metadata = await fetched_metadata
---
<html lang="en">

<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width" />
	<title>{metadata.siteTitle}</title>
</head>

<body>
	<h1>{metadata.siteTitle}</h1>
</body>

</html>

/src/fetched.js:

const fetched_metadata = fetch('http://localhost:3000/metadata.json').then(
	response => response.json()
)

export { fetched_metadata }

/public/metadata.json:

{
  "siteTitle": "Astro"
}

However, if I move the const fetched_metadata logic out of /src/fetched.js and into /src/pages/index.astro it will work.

And then, if I undo that change, it will keep working!

It’s as if using fetch is not allowed in external .js files, until it gets used in an .astro file, at which point it gets put into the global scope and becomes available to the .js files or something.

Steps to Reproduce

  1. Install astro, or create an astro project using npm init astro.
  2. Follow the steps outlined above.
  3. See the errors described above, and the work-around described above.

Link to Minimal Reproducible Example (Optional)

https://github.com/jonathantneal/astro-fetch-outside-astro-issue
@drwpow drwpow added the help wanted Please help with this issue! label Aug 16, 2021
@drwpow
Copy link
Member

drwpow commented Aug 16, 2021 •
edited
Loading

This is definitely a bug. Should be as simple as injecting isomorphic-fetch into all .js files as we do .astro files (example).

@itday
Copy link

itday commented Aug 19, 2021

There is an article in the Astro documentation: Data Fetching#using-fetch-outside-of-astro-components.

Add import fetch from 'node-fetch'

@matthewp
Copy link
Contributor

If anyone wants to work on this, it's probably just a matter of adding code to the transform step here: https://github.com/snowpackjs/astro/blob/d0e7fcfc0652c8e5b6af0b584838b605efb92a8a/packages/astro/snowpack-plugin.cjs#L27

If the file is .js or .ts add the fetch import. This will make it available in all JS files.

@matthewp
Copy link
Contributor

Actually, the above is not a good solution, this will also affect frontend code!

Probably the simple way is better and just add globalThis.fetch = fetch in the runtime.

@johnhenry
Copy link
Contributor

johnhenry commented Aug 25, 2021 •
edited
Loading

Should the fix be:
a) Ensure that "fetch" is consistently present in all build-time .js files OR
b) Prevent "fetch" ever being automatically present, forcing the user to use "node-fetch" in these situations.

I believe that a) would be fun and convenient, but b) -- having to import fetch -- wold be consistent with the rest of the node. As @itday mentions above, this already seems expected in the documentation.

(Tangentially, deno has built-in 'fetch'. In this case, a) would be consistent. I wonder if there's been any thought toward selecting build-time environment: e.g. astro build --buildtime=deno.)

@matthewp
Copy link
Contributor

@johnhenry Global fetch was already accepted via the RFC process here: #410 . So this is a bug.

The solution here is to make fetch available on the globalThis object, rather than trying to make it an import like the current implementation. This will ensure that it's always available in all files.

@matthewp
Copy link
Contributor

Here's a quick solution: f22a1cc

@natemoo-re natemoo-re mentioned this issue Oct 15, 2021
Merged
4 tasks
@natemoo-re
Copy link
Member

Fixed in #1406!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Please help with this issue!
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@jonathantneal @matthewp @johnhenry @drwpow @natemoo-re @itday