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

Upgrading astro, astro-storyblok, and netlify adapter results in build error in Netlify (SSR) #5996

Closed
1 task done
SandraRodgers opened this issue Jan 26, 2023 · 6 comments · Fixed by #6117
Closed
1 task done
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@SandraRodgers
Copy link

What version of astro are you using?

2.0.0

Are you using an SSR adapter? If so, which one?

Netlify

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

I updated my project to Astro 2.0. I'm using the netlify adapter for SSR. The dependencies have also been updated:

"dependencies": {
    "@astrojs/netlify": "^2.0.0",
    "@astrojs/tailwind": "^3.0.0",
    "@storyblok/astro": "^2.0.5",
    "astro": "^2.0.1",
    "tailwindcss": "^3.2.4"
  }

For netlify builds, Netlify is running Node v16.19.0.

When I run it locally, the site builds fine. In storyblok I can edit and publish and see the updates, both the visual editor and on my localhost.

Previously, it would build fine when I pushed to main (before I upgraded to Astro 2.0)

Now, when I push my updated project to main, the netlify deployment happens. It builds the site, but results in an error.

Attaching screenshot of error and build logs where related warning happens

Screen Shot 2023-01-26 at 12 33 06 PM

Screen Shot 2023-01-26 at 1 52 07 PM

Link to Minimal Reproducible Example

https://github.com/SandraRodgers/astro-storyblok-example

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

@Princesseuh when we removed Node 14 support did was stop shimming fetch and Headers globals? We might need to bring that back if so, since we document using things like new Headers() in code.

@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Jan 26, 2023
@Princesseuh
Copy link
Member

@Princesseuh when we removed Node 14 support did was stop shimming fetch and Headers globals? We might need to bring that back if so, since we document using things like new Headers() in code.

No, we still provide those through undici. https://github.com/withastro/astro/blob/main/packages/webapi/src/ponyfill.ts#L74-L78

@natemoo-re
Copy link
Member

Yeah I looked into this a bit and nothing obvious jumped out, webapi was the first place I looked and it all looked OK to me.

@matthewp
Copy link
Contributor

The polyfill is applied here: https://github.com/withastro/astro/blob/main/packages/integrations/netlify/src/netlify-functions.ts#L6

I could be a bundling issue where the code using Headers happens at the module scope and it higher in the bundled file than the shimming happens. If that's the case then we need to massage the bundling order so that the shimming happens first.

But it could also be caused by something else.

@Princesseuh
Copy link
Member

Princesseuh commented Jan 31, 2023

Confirming that it's a polyfill thing, the reason it worked before is just a coincidence (node-fetch put Headers and Response in the global scope, undici doesn't), as the polyfill is still done the same way.

We need to figure out a way to include our polyfill at the start of the generated entry.mjs file instead of at the end. Using Node 18 to build the website does work just fine though, since Headers and Request are both naturally available there

@philr35
Copy link

philr35 commented Feb 4, 2023

Also having this issue, same setup as @SandraRodgers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants