-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(devserver) Remove "prod" requirement for using publicPath #4962
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
Conversation
@EEParker I don't believe they shouild match. Very rarely does a developer computer match the same set up as a production server. |
@hawkeye64 perhaps not, and in my test case with aspdotnet I am using different paths. However, within my test project where quasar is a "child" application, the base tag does needs to match what aspdotnet expects the route to be to work. This patch allows it to work in development and hmr to function. |
@EEParker I am just saying don't assume they should match. Based on this: // this should match the publicPath from above |
Hi @EEParker , Thanks for contributing! I cannot merge this as it would break some Quasar modes. However, what we could do is setup a new Boolean flag under quasar.conf.js > build, name it "forceDevPublicPath", then scenarios like yours can be covered too. I'm just wondering over the very large implications of this. Were you able to test the changes in this PR? Need help getting you up to speed on testing CLI changes? Also, would need some doc updates for the new flag. Again, I can show you quickly how to do it (ping me on Discord) -- if this is unclear how to do. Should be a 1-2 minute thing to add this to docs. |
@rstoenescu Thanks for the feedback. I did test spa and pwa modes, however I did not test Electron or Cordova builds or really consider the implications. Your suggestion of a |
@@ -711,7 +712,7 @@ class QuasarConfig { | |||
} | |||
} | |||
else { | |||
cfg.build.env.__statics = `"${this.ctx.dev ? '/' : cfg.build.publicPath || '/'}statics"` | |||
cfg.build.env.__statics = `"${((this.ctx.prod || cfg.build.forceDevPublicPath) && cfg.build.publicPath) ? cfg.build.publicPath : '/' }statics"` |
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 switched the logic here to match L478.
Merged it and pushed a subsequent commit to bulletproofed it. @EEParker Thanks for starting this and taking time for it for a PR! Much appreciate it. |
…framework#4962) * add option to force publicPath in 'dev' builds * Update quasar-conf-js.md
This PR allows
publicPath
and thebase
tag to work on dev builds. It is used in conjunction with webpackDevServer publicPath.What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
Fixes #4840, #4687
quasar.conf.js