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

fix(cloudflare): target es2022 instead of es2020 to fix esbuild incompatibility issues #8682

Merged
merged 10 commits into from
Sep 28, 2023

Conversation

dario-piotrowicz
Copy link
Contributor

@dario-piotrowicz dario-piotrowicz commented Sep 27, 2023

Changes

The Cloudflare adapter currently uses esbuild targeting es2020, this works fine but this can be problematic when used with some astro integrations, like @astrojs/mdx, the issue being that mdx seems to use top-level awaits not supported by es2020

So if I have an astro config such as this:

export default defineConfig({
  site: 'https://example.com',
  integrations: [mdx(), sitemap()],
  output: "server",
  adapter: cloudflare(),
});

and try to build my application I get the following:
Screenshot 2023-09-27 at 19 48 40

Note
This happens with the blog template, if I start a brand new application, as soon as I add the Cloudflare adapter and try building my application I get this error

Note
This might also break in other cases as well, this with the mdx integration is the only one I noticed thus far

Testing

I'm not sure if such a small change requires tests, please let me know if I should look into adding those

Docs

There isn't really any documentation to add as this is a simple change that should not be visible to users.

@changeset-bot
Copy link

changeset-bot bot commented Sep 27, 2023

🦋 Changeset detected

Latest commit: a94243c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Sep 27, 2023
@matthewp
Copy link
Contributor

cc @alexanderniebuhr

@alexanderniebuhr alexanderniebuhr self-requested a review September 28, 2023 06:24
Copy link
Member

@alexanderniebuhr alexanderniebuhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dario-piotrowicz Fantastic work! 🚀

I've verified that CF Workers & Pages support es2022. Given that all existing tests have passed, there's no need for additional tests. Documentation updates aren't necessary either.

@dario-piotrowicz
Copy link
Contributor Author

Thanks a lot @alexanderniebuhr ❤️, and thanks for cleaning up the PR and adding the one spot I missed 🙈

@alexanderniebuhr
Copy link
Member

@dario-piotrowicz Checking in on this. I'd like to confirm if the documentation team has reviewed the changeset. Once that's done, this will be ready for merge, and I'll expedite the process.

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs approves the changeset! 🥳

@alexanderniebuhr alexanderniebuhr merged commit c3572fd into withastro:main Sep 28, 2023
13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants