-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add the output
option
#4015
Add the output
option
#4015
Conversation
🦋 Changeset detectedLatest commit: 3d179ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
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 |
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.
LGTM so far!
force people to rename adapter to deploy or stay back-compat with adapter and a deprecation notice?
I'd lean for back-compat + deprecation notice. It's cheap to do a config rename like that inside of validateConfig()
iirc.
TODO:
|
4acd885
to
fa48c4e
Compare
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.
Blocking this PR. There is an ongoing discussion on Discord about overloading the name mode
which is a well-defined Vite-ism.
As soon as that's resolved, I'm totally on board with the core changes here.
1e97fdf
to
a5ffe45
Compare
.changeset/famous-coins-destroy.md
Outdated
'@astrojs/vercel': minor | ||
--- | ||
|
||
New `mode` configuration option |
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.
Changeset needs to be updated to refer to output
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.
Thanks for helping to get this over the line @matthewp. I took a quick pass to clean this up for you so that we can merge in time for the RC. Here's a summary of the changes made on top of your PR:
- cleaned up some of the auto-doc generation types.
- cleaned up some of the old code from my original PR that had made its way into this one (old changesets, old "exports' entry, etc)
- Kept the config
output
and the term "output target"/"output mode" used to describe it. - Reverted the
deploy
change to keep it asadapter
for now. I'm glad that we experimented with a new name here but after trying a few out first-hand I wasn't happy with anything to the point that it would be worth the switch and increased migration cost of the more importantoutput
change. Let's keep it as "adapter" for now and can revisit later if user feedback is that the word "adapter" is still too vague.
Changes
output: 'static' | 'server'
output === 'server'
we through an error during build.Testing
Docs