fix(fullstack): Allow configuration of the public_path #3419
Conversation
|
I haven't tried to update the CLI tool, possible that should be updated also. Not sure. Haven't thought that far ahead yet. |
|
Tested this change now using real code and it is working. |
|
Could also replace But being able to customize value from application I think is good also. |
jkelleyrtp
left a comment
There was a problem hiding this comment.
Overall looks good but adding the public_path into the signatures is semver breaking.
I think it'd be better to read an env var instead since I'd rather change the public_dir depending on my deploy setup rather than at compile time.
packages/fullstack/src/server/mod.rs
Outdated
| /// } | ||
| /// ``` | ||
| fn serve_static_assets(self) -> Self | ||
| fn serve_static_assets(self, pubic_path: &Path) -> Self |
There was a problem hiding this comment.
Modifying this signature is not semver compatible meaning we wouldn't be able to release this in version 0.6.2.
I think we can add this with a default impl but since DioxusRouterExt is not sealed, any changes to its signature will need to follow semver rules.
https://doc.rust-lang.org/cargo/reference/semver.html#trait-item-signature
|
OK. I must admit I never considered being semver compatible. Unless any objections, I will revert these changes for one that replaces |
c62b587 to
26132cf
Compare
2a881ef to
f224266
Compare
f224266 to
fcbdd5a
Compare
This was annoying me. I don't think the default is always sensible.
I ran the tests and they passed, but haven't yet tried running the code.