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: use server.perEnvironmentStartEndDuringDev #18549

Merged

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Nov 1, 2024

Description

server.perEnvironmentStartEndDuringDev was not used anywhere.

refs #18491 (comment)

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: environment API Vite Environment API labels Nov 1, 2024
Comment on lines 521 to 525
plugin.perEnvironmentStartEndDuringDev !== true,
config.server.perEnvironmentStartEndDuringDev ||
plugin.perEnvironmentStartEndDuringDev === true,
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess this was opposite.

Copy link
Member

Choose a reason for hiding this comment

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

boolean conditions are the hardest ones 🤦🏼

patak-dev
patak-dev previously approved these changes Nov 1, 2024
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

To be honest, I didn't understand your linked question. I thought we only had this option at the plugin level and not a way to enable it for all plugins. This makes sense to me though.

await this.hookParallel(
'buildEnd',
(plugin) => this._getPluginContext(plugin),
() => [],
(plugin) =>
this.environment.name === 'client' ||
plugin.perEnvironmentStartEndDuringDev !== true,
config.server.perEnvironmentStartEndDuringDev ||
plugin.perEnvironmentStartEndDuringDev === true,
Copy link
Member

Choose a reason for hiding this comment

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

We can replace both plugin.perEnvironmentStartEndDuringDev === true with plugin.perEnvironmentStartEndDuringDev too, no?

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 required expanding hookParallel's type a bit, but it was possible 👍

@patak-dev patak-dev merged commit fe30349 into vitejs:main Nov 1, 2024
15 checks passed
@sapphi-red sapphi-red deleted the fix/use-server-per-env-start-end-during-dev branch November 1, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: environment API Vite Environment API p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants