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(vercel): copy static assets after all integrations #508

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ascorbic
Copy link
Contributor

Changes

Adapters need to run before all other integrations, which is enforced in builds. This causes a problem for the Vercel adapter, which copies static assets from the dist folder into the .vercel/static folder after the build in the 'astro:build:done' hook. For integrations that also generate assets in 'astro:build:done' (such as sitemap and partytown), this means that they are too late, and generate the assets after the adapter has copied them to the deploy dir.

This PR does a bit of a sneaky trick: it adds a second integration, which isn't an adapter, and so can run after all others. That has its own 'astro:build:done' hook which does the copying.

Fixes #445

Testing

Added fixture and test suite

Docs

Copy link

changeset-bot bot commented Jan 17, 2025

🦋 Changeset detected

Latest commit: 83ba9f9

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

This PR includes changesets to release 20 packages
Name Type
@astrojs/vercel Patch
@test/astro-vercel-basic Patch
@test/astro-vercel-image Patch
@test/astro-vercel-integration-assets Patch
@test/vercel-isr Patch
@test/vercel-max-duration Patch
@test/vercel-edge-middleware-with-edge-file Patch
@test/vercel-edge-middleware-without-edge-file Patch
@test/astro-vercel-no-output Patch
@test/astro-vercel-prerendered-error-pages Patch
@test/astro-vercel-redirects-serverless Patch
@test/astro-vercel-redirects Patch
@test/vercel-server-islands Patch
@test/astro-vercel-serverless-prerender Patch
@test/astro-vercel-serverless-with-dynamic-routes Patch
@test/astro-vercel-static-assets Patch
@test/astro-vercel-static Patch
@test/vercel-streaming Patch
@test/astro-vercel-with-web-analytics-enabled-output-as-static Patch
vercel-hosted-astro-project Patch

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: vercel Related to Vercel adapter (scope) label Jan 17, 2025
Copy link
Member

@florian-lefebvre florian-lefebvre left a comment

Choose a reason for hiding this comment

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

LGTM. Only issue I can see is that another integration could be added after this one from within an integration:

  • adapter adds integration X
  • integration Y adds integration Z
  • X runs (adapter assets copy)
  • Z runs

So if Z adds some assets they will not be copied. This should be rare tho. Maybe a way to work around that is for the adapter to inject an integration that injects an integration:

  • adapter adds integration X
  • integration Y adds integration Z
  • X adds integration W
  • Z runs
  • W runs (adapter assets copy)

This is not bullet proof of course

@ascorbic
Copy link
Contributor Author

But what if another integration injects an integration that injects an integration? Maybe we should inject an integration that injects an integration that injects an integration that injects an integration

@florian-lefebvre
Copy link
Member

Lol I'm not sure if you're joking but I'll assume you don't. I'd say it's quite common for an integration to inject another (eg. to organize code mainly) so handling one level would be enough. But I'm fine with the current fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: vercel Related to Vercel adapter (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@astrojs/sitemap excluded from final static output with Vercel adapter
2 participants