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.

Sign up for GitHub

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: do not override preset when prerendering #605

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

fix: do not override preset when prerendering #605

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afac9de
fix: handle prerender case for preset env variables
BobbieGoede Dec 19, 2024
fd2e65b
fix: use correct variable
BobbieGoede Dec 19, 2024
6e9aeb8
fix: fallback to static preset for prerender if unset
BobbieGoede Dec 20, 2024
15424a1
fix: do not override `preset`
BobbieGoede Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/commands/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ export default defineCommand({

const kit = await loadKit(cwd)

const nitroPreset = ctx.args.prerender ? 'static' : ctx.args.preset || process.env.NITRO_PRESET || process.env.SERVER_PRESET
const resolvedNitroPreset
= ctx.args.preset || process.env.NITRO_PRESET || process.env.SERVER_PRESET
const nitroPreset = ctx.args.prerender ? 'static' : resolvedNitroPreset
Copy link
Member

Choose a reason for hiding this comment

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

in fact this is still not quite right - there are several static presets, including vercel-static, netlify-static, and so on, and we probably need to ignore (that is, not log warnings) for any static preset that's passed

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like nitro tries to infer the correct static preset based on the value of preset when static: true (nitrojs/nitro#2860) it's undocumented though (nitrojs/nitro#1524). I just removed the logging and only fall back to static preset if none is provided during prerender, is this what you had in mind?

Copy link
Member

Choose a reason for hiding this comment

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

we should not set the static preset but instead pass static: true

you can test by setting (for example) NETLIFY=1 nuxi build --prerender - we want it in that case to use netlify-static preset.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I think/hope I finally understand, I removed the overriding of the preset variable and now log the resolved nitro preset instead.

if (nitroPreset) {
if (ctx.args.prerender && ctx.args.preset) {
consola.warn(`\`--prerender\` is set. Ignoring \`--preset ${ctx.args.preset}\``)
if (ctx.args.prerender && resolvedNitroPreset) {
consola.warn(
`\`--prerender\` is set. Ignoring \`--preset\`, \`NITRO_PRESET\`, \`SERVER_PRESET\` (${resolvedNitroPreset})`,
)
}
// TODO: Link to the docs
consola.info(`Using Nitro server preset: \`${nitroPreset}\``)
Expand Down
Loading