-
Notifications
You must be signed in to change notification settings - Fork 365
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: respect build plugins changing build.publish
#6133
Conversation
src/utils/run-build.mjs
Outdated
@@ -123,6 +123,7 @@ export const runNetlifyBuild = async ({ command, env = {}, options, settings, ti | |||
await devCommand({ | |||
command: undefined, | |||
useStaticServer: true, | |||
dist: options.dir ? undefined : netlifyConfig?.build?.publish, |
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.
Hmm this feels a bit odd. Is it because options.dir
takes precedence and devCommand
will find it on its own? If so, would it make sense to always send dist
but make it prefer options.dir
over netlifyConfig?.build?.publish
?
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.
Agree it feels odd. But, always sending dist
will effectively override all settings that come out of detectServerSettings
, so i'd prefer not to do that.
Now that I think about it, undefined
will also override dist
because it's being destructured in devCommand
- gonna need to change this.
While working on netlify/angular-runtime#67, I noticed we're not respecting build plugins changing
build.publish
. We should! This PR fixes that.