-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
refactor(server): move publicPath configuration from CLI to API #2098
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2098 +/- ##
==========================================
+ Coverage 94.46% 94.49% +0.03%
==========================================
Files 32 33 +1
Lines 1210 1218 +8
Branches 334 337 +3
==========================================
+ Hits 1143 1151 +8
Misses 65 65
Partials 2 2
Continue to review full report at Codecov.
|
@@ -58,6 +59,8 @@ class Server { | |||
this.compiler = compiler; | |||
this.options = options; | |||
|
|||
setPublicPath(compiler, options); |
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.
Let's move this in updateCompiler
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.
@evilebottnawi this is not altering the compiler at all though, only using the compiler's option data to set devServer.publicPath
. But if you still think it should be there I have no strong opinion.
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.
I am afraid what is breaking change
/cc @hiroppy
This pr should be changed to the next. |
I think I will just open new PR on next |
Closing in favor of #2099 |
For Bugs and Features; did you add new tests?
Not yet
Motivation / Use-Case
This is part of the effort to move configuration changes out of the CLI and into the API. The goal here is to set the
publicPath
option in the API if it is not already set.Breaking Changes
No breaking changes on CLI.
On API, if a user was not specifying a
publicPath
in the past, it would not be set. Now, it will be set based oncompiler.options.output.publicPath
, if it exists. Should this PR be onnext
for this reason?Additional Info