prefix-path with minimal changes
Related PLIP #https://github.com/plone/volto/issues/4290
The idea is to use React-router basename which takes care of 90% of our tasks. The only caveat is how it handles the root path when basename is set. By default <Link to='/'/> will point to /prefix/ (with a trailing slash). RRv6 has a fix for this but its not backported to v5.x.
Deploy Preview for plone-components canceled.
| Name | Link |
|---|---|
| Latest commit | 998e8fb2b12828bc0ae814b95fc2883e3d075a8c |
| Latest deploy log | https://app.netlify.com/sites/plone-components/deploys/67ea7af7b1ed52000869dfc8 |
We need to port the Cypress tests, right? I guess that then we will assess more things in there.
@sneridagh I think the tests won't work, before everything in #3592 is here, or in other PRs. See: https://github.com/plone/volto/pull/3592#issuecomment-2754312281
@wesleybl well, the idea is to have the minimum in here that works, we won't know what does work and what fails, and what's left until we do it.
@sneridagh So should we bring the tests here and see what fails?
@wesleybl yeah, I would do that. I don't know if @nileshgulia1 is on it already, please ask him just to make sure you don't step on each other.
@wesleybl yeah, I would do that. I don't know if @nileshgulia1 is on it already, please ask him just to make sure you don't step on each other.
@sneridagh @nileshgulia1 I can do this in another PR.
I've been testing this some today. If Volto is on a subpath http://somehost/foo, then I would expect that there is often a different website served at http://somehost. So, in seamless mode we should try to also serve the API under the prefix, at http://somehost/foo/++api++, instead of at http://somehost/++api++
I am experimenting with doing this in a new branch: https://github.com/plone/volto/compare/prefix-path-revised...davisagli-prefix-path?expand=1
I've been testing this some today. If Volto is on a subpath http://somehost/foo, then I would expect that there is often a different website served at http://somehost/. So, in seamless mode we should try to also serve the API under the prefix, at http://somehost/foo/++api++, instead of at http://somehost/++api++
I do this with Nginx. Do you mean that Volto itself should do this?
I am experimenting with doing this in a new branch: https://github.com/plone/volto/compare/prefix-path-revised...davisagli-prefix-path?expand=1
@davisagli I tested your branch and it worked! Requests are made to /foo/++api++
I am experimenting with doing this in a new branch: https://github.com/plone/volto/compare/prefix-path-revised...davisagli-prefix-path?expand=1
@davisagli I tested your branch and it worked. Do you plan to make a PR? It would be very welcome!
@wesleybl Thanks for the reminder. I'll try to find time for it soon.
Really impressive! In my initial tests everything just worked fine. Just one note for the documentation: in production the build needs to be done with the environment variables set. For example:
RAZZLE_PREFIX_PATH=/testme pnpm build
NODE_ENV=production RAZZLE_PREFIX_PATH=/testme node build/server.js
Otherwise, the built assets will be missing the prefix, and the site will try to load JavaScript files from /static/assets/js/client..js instead of /testme/static/....
Thereβs an edge case with prefix paths like /foo/bar that requires a fix in devproxy if you want to support it. This solution works for me.
diff --git a/packages/volto/src/express-middleware/devproxy.js b/packages/volto/src/express-middleware/devproxy.js
index e219edfd8..123a97fb8 100644
--- a/packages/volto/src/express-middleware/devproxy.js
+++ b/packages/volto/src/express-middleware/devproxy.js
@@ -85,7 +85,7 @@ export default function devProxyMiddleware() {
config.settings.proxyRewriteTarget ||
`/VirtualHostBase/${apiPathURL.protocol.slice(0, -1)}/${
apiPathURL.hostname
- }:${apiPathURL.port}${instancePath}/++api++/VirtualHostRoot${config.settings.prefixPath ? '/_vh_' + config.settings.prefixPath.slice(1) : ''}`;
+ }:${apiPathURL.port}${instancePath}/++api++/VirtualHostRoot${config.settings.prefixPath ? config.settings.prefixPath.split('/').filter(Boolean).map(part => '/_vh_' + part).join('') : ''}`;
return `${target}${path.replace(`${config.settings.prefixPath}/++api++`, '')}`;
},
@mamico
Just one note for the documentation: in production the build needs to be done with the environment variables set.
I tried to find a way to avoid this, but it's not trivial. Setting __webpack_public_path__ in the entry point makes it mostly work, but then I'm still having trouble with the CSS files which are extracted at build time using mini-css-extract-plugin. I suggest we document this as is, and maybe see if we can improve it later.
Thereβs an edge case with prefix paths like /foo/bar that requires a fix in devproxy if you want to support it. This solution works for me.
Thanks, I added this.
@mamico
Just one note for the documentation: in production the build needs to be done with the environment variables set.
I tried to find a way to avoid this, but it's not trivial...
Ok. I think could be not a problem. Need only to be documented.
Thereβs an edge case with prefix paths like /foo/bar ...
Thanks, I added this.
Thanks
A last one, do we need a special documentation note on production deployments?
A final one: Don't we have to amend also the middlewares? eg. robots.txt?
Yes, it is necessary to prefix middleware. See:
https://github.com/plone/volto/pull/3592/commits/5ae6259dc833f5eab06beb73bee2f36a524cb005
At the time, I had problems prefixing devProxyMiddleware. Another thing that shouldn't be prefixed is /ok. If Volto is in a container, and we're monitoring Volto, the monitoring doesn't know what the prefix is.
@sneridagh @wesleybl I think the middlewares are all handled correctly. Some of them like the robots.txt one match on a path starting with **/, so it doesn't matter what the prefix is.
I think the middlewares are all handled correctly. Some of them like the robots.txt one match on a path starting with **/, so it doesn't matter what the prefix is.
@davisagli I remember that when we added the portal logo through the control panel, we needed to prefix some middleware for it to work. We need to test this. In any case, I prefer to prefix the middleware, in case some new middleware is created, or an addon creates one.
A last one, do we need a special documentation note on production deployments?
@sneridagh I thought I covered this here:
https://github.com/plone/volto/pull/6897/files#diff-feef314560bd269daadb31268babb163ecab44efa5b18b9d575276ac254417f6R22
@davisagli I remember that when we added the portal logo through the control panel, we needed to prefix some middleware for it to work. We need to test this.
@wesleybl Aha, fixed in https://github.com/plone/volto/pull/6897/commits/a94fe3dfe6e4e8abb8abcbaec593dee303f0003f
In any case, I prefer to prefix the middleware, in case some new middleware is created, or an addon creates one.
I see your point. On the other hand, if we don't prefix all the middleware when it is used, then it's easier to handle the exceptions where the middleware shouldn't have a prefix. I would personally like to merge this as is, and then consider this in a separate PR.
I see your point. On the other hand, if we don't prefix all the middleware when it is used, then it's easier to handle the exceptions where the middleware shouldn't have a prefix. I would personally like to merge this as is, and then consider this in a separate PR.
@davisagli I still don't see any advantage in not prefixing. I think most cases should be prefixed. Actually, I can only think of /ok and devProxyMiddleware. I think we'll make fewer mistakes if we prefix. If I didn't know about this PR and wanted to create middleware that only made sense in the context of the portal root, I would create something like /my-middleware, not **/my-middleware. This doesn't include middleware that might already be created in addons.
But this isn't a block. We can continue like this if you want and discuss it in another issue.
@wesleybl I was thinking about this after my comment and also realizing that it would make it easier to add custom middleware if you don't have to think about the prefix there. So I think we should give it a try the way you showed in https://github.com/plone/volto/commit/5ae6259dc833f5eab06beb73bee2f36a524cb005 -- I just don't want to delay merging this much longer.
A last one, do we need a special documentation note on production deployments?
@sneridagh I thought I covered this here:
https://github.com/plone/volto/pull/6897/files#diff-feef314560bd269daadb31268babb163ecab44efa5b18b9d575276ac254417f6R22
@davisagli I was referring to actual deployment example, as in Docker/Traefik config.
I was referring to actual deployment example, as in Docker/Traefik config.
I'm planning to add a demo site once this is merged and released. I'd like to use that to confirm we have all the pieces right*; then we can add it to the docs and/or point to the demo as an example.
*(It's mostly just setting the environment variable, but we probably have to adjust the traefik rules too)
I'm planning to add a demo site once this is merged and released. I'd like to use that to confirm we have all the pieces right*; then we can add it to the docs and/or point to the demo as an example.
*(It's mostly just setting the environment variable, but we probably have to adjust the traefik rules too)
Makes sense to me. Would you please create a new issue, and self-assign?
@davisagli Thank you for taking it forward. I took care of renaming things as per @stevepiercy's suggestions. Let's go go go...π
@nileshgulia1 Thank you, I really appreciate it. I'll check it later today.
@datakurre @rioksane did you recently add security to your network to block certain requests to https://tech.blog.jyu.fi/2020/06/plone-volto-hasura-gatsbyjs-mashup/ ? The page serves fine under a GET request, but Lychee, our README linkchecker, reports a network error https://github.com/plone/volto/actions/runs/18709350106/job/53353804838?pr=6897#step:3:98.
@datakurre @rioksane I ran through the Lychee troubleshooting guide at https://lychee.cli.rs/troubleshooting/network-errors/ and it appears that your certificate is not valid or recognized per https://lychee.cli.rs/troubleshooting/network-errors/#use-the---insecure-flag.
$ lychee --insecure packages/volto/README.md
126/126 ββββββββββββββββββββ Finished extracting links Issues found in 1 input. Find details below.
[packages/volto/README.md]:
[403] https://www.npmjs.com/package/@plone/client | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/components | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/helpers | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/providers | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/registry | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/scripts | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/types | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/volto | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/package/@plone/volto-slate | Rejected status code (this depends on your "accept" configuration): Forbidden
[403] https://www.npmjs.com/search?q=keywords%3Avolto-addon%2Cvolto | Rejected status code (this depends on your "accept" configuration): Forbidden
π 126 Total (in 5s) β
116 OK π« 10 Errors
To resolve the issue, see https://lychee.cli.rs/troubleshooting/network-errors/#update-the-ca-certificates.