-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update to astro v5 #2
base: master
Are you sure you want to change the base?
Conversation
@@ -44,9 +40,7 @@ function getAdapter({ | |||
name: baseConfig.name, | |||
supportedAstroFeatures: { | |||
...baseConfig.supportedAstroFeatures, | |||
assets: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fwang I'm pretty sure this line is why my build fails due to lack of sharp support. I kept sharpImageService: "unsupported"
in this MR for parity, but I'm confused: I'm able to deploy on Astro v4 with the current astro plugin. Changing this the value to stable
fails for a different reason, which makes sense if there actually isn't support.
Do you mind providing some context into sharp support on static sites?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the Astro build output already have sharp
bundled in node modules? I vaguely remember it is.
(/cc @nerdoza)
In the context of Lambda, sharp is normally bundled like this https://sst.dev/docs/examples/#sharp-in-lambda
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Their docs make it seem like it is:
https://docs.astro.build/en/guides/images/#default-image-service
I use pnpm, so I also have sharp added as a dependency. That said, I'm still observing the issue with the passthrough image service. I'll do more testing here to confirm.
As the library owner, would you expect that sharp would be supported with statics sites? It seems like it was, since I was using it, but the configuration in the current adapter code makes it seem like it wasn't/shouldn't. I'm also not deep with Astro adapters, so it's also possible I'm missing something.
@san4d : in the "src/entrypoint.ts" there is also a deprecated import Going to test a bit today... Will comment whatever I find. |
Thanks, Michael. Not sure how I missed that in the build. I replaced it with a call from astro core, which I'll add to this MR's description. |
@@ -24,17 +24,13 @@ function getAdapter({ | |||
exports: ["handler"], | |||
adapterFeatures: { | |||
edgeMiddleware: false, | |||
functionPerRoute: false, | |||
buildOutput: isStatic ? "static" : "server", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fwang This is another change I wanted to highlight. Without this, I believe the buildOutput
would default to server
. I'm not sure if that's a bug or expected behavior. As a consumer of the API, I'd expect the output to be static if that's what I passed in.
I think we can also:
|
Summary
edge
deployment strategy3.0.0-beta
). This happens to match the SST major version and reflects breaking changes to the API. The beta status reflects the absence of wide testing.Related Breaking Changes
Out of Scope
builtResult.routes
parameterOutstanding Issues