Skip to content

feat: publicPath output#5434

Open
adubrouski wants to merge 2 commits intowebpack:mainfrom
adubrouski:publicPath-output
Open

feat: publicPath output#5434
adubrouski wants to merge 2 commits intowebpack:mainfrom
adubrouski:publicPath-output

Conversation

@adubrouski
Copy link

@adubrouski adubrouski commented Mar 16, 2025

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

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:

output: {
  publicPath: "/my-public-path/",
}

We getting this:

<i> [webpack-dev-server] Project is running at:
<i> [webpack-dev-server] Loopback: http://localhost:8080/, http://[::1]:8080/
<i> [webpack-dev-server] On Your Network (IPv4): http://10.252.1.16:8080/
<i> [webpack-dev-server] Content not from webpack is served from '/home/user/Project/src' directory
<i> [webpack-dev-server] 404s will fallback to '/index.html'

My proposal is:

<i> [webpack-dev-server] Project is running at:
<i> [webpack-dev-server] Loopback: http://localhost:8080/my-public-path/, http://[::1]:8080/my-public-path/
<i> [webpack-dev-server] On Your Network (IPv4): http://10.252.1.16:8080/my-public-path/
<i> [webpack-dev-server] Content not from webpack is served from '/home/user/Project/src' directory
<i> [webpack-dev-server] 404s will fallback to '/index.html'

Breaking Changes

No

Additional Info

No

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 16, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

alexander-akait
alexander-akait reviewed Mar 24, 2025
View reviewed changes
lib/Server.js
hostname: newHostname,
port,
pathname:
typeof publicPath === "function" || publicPath === "auto"
Copy link
Member

@alexander-akait alexander-akait Mar 24, 2025

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)

Copy link
Author

@adubrouski adubrouski Mar 25, 2025

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?

alexander-akait
alexander-akait reviewed Mar 26, 2025
View reviewed changes
lib/Server.js
this.logger.info(
`You probably have a custom public path, please navigate in your browser to the point that you required`,
);
}
Copy link
Member

@alexander-akait alexander-akait Mar 26, 2025

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

Copy link
Author

@adubrouski adubrouski Mar 27, 2025

Choose a reason for hiding this comment

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

@alexander-akait

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

Copy link
Member

@alexander-akait alexander-akait Apr 2, 2025

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

Copy link
Author

@adubrouski adubrouski Apr 2, 2025 •
edited
Loading

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?

alexander-akait
alexander-akait reviewed Apr 4, 2025
View reviewed changes
Copy link
Member

@alexander-akait alexander-akait left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-akait alexander-akait alexander-akait left review comments

@hiroppy hiroppy Awaiting requested review from hiroppy

@snitin315 snitin315 Awaiting requested review from snitin315

@anshumanv anshumanv Awaiting requested review from anshumanv

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adubrouski @alexander-akait