-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
feat: publicPath output #5434
base: master
Are you sure you want to change the base?
feat: publicPath output #5434
Conversation
e8e5757
to
ba4acf1
Compare
hostname: newHostname, | ||
port, | ||
pathname: | ||
typeof publicPath === "function" || publicPath === "auto" |
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.
We need respect function
too... But yeah, we known about it only after compilation, so maybe we can improve our message and write - something like you can have custom public path, please navigate in your browsers to the point that you required
(just an example, feel free to improve it)
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.
@alexander-akait I think adding an info message if public path is auto
or function
would be a good idea, can you check this commit?
a6715c6
to
ba4acf1
Compare
this.logger.info( | ||
`You probably have a custom public path, please navigate in your browser to the point that you required`, | ||
); | ||
} |
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.
We should output this only for function
, auto
is like /
, so it is unnecessary
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, webpack documentation says that the "auto" value allows you to automatically infer the public path value based on the values in the scripts. Are you sure that we should keep default behavior for "auto" value?
https://webpack.js.org/guides/public-path/#automatic-publicpath
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.
Yes, you are right, but mostly it used only for /
, so I think it can be a little bit spammy
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.
Ok, I fixed it in this commit. Can you check?
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.
Looks good, I will look deeply before the next release, thank you
For Bugs and Features; did you add new tests?
Yes
Motivation / Use-Case
Some applications use public paths (to avoid strong differences between dev and prod builds). I set it via output.publicPath, and it seems a bit inconvenient to me that the urls in the terminal are obtained without taking publicPath into account. These can be simple applications without routing, and you have to go to / and then manually add publicPath. I suggest outputting the url with publicPath if it is set statically, if it is determined on the fly - output without it.
Now, with config like this:
We getting this:
My proposal is:
Breaking Changes
No
Additional Info
No