volto icon indicating copy to clipboard operation
volto copied to clipboard

prefix-path with minimal changes

Open nileshgulia1 opened this issue 9 months ago β€’ 6 comments

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.

nileshgulia1 avatar Mar 28 '25 08:03 nileshgulia1

Deploy Preview for plone-components canceled.

Name Link
Latest commit 998e8fb2b12828bc0ae814b95fc2883e3d075a8c
Latest deploy log https://app.netlify.com/sites/plone-components/deploys/67ea7af7b1ed52000869dfc8

netlify[bot] avatar Mar 28 '25 08:03 netlify[bot]

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 avatar Apr 09 '25 15:04 wesleybl

@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 avatar Apr 09 '25 15:04 sneridagh

@sneridagh So should we bring the tests here and see what fails?

wesleybl avatar Apr 09 '25 15:04 wesleybl

@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 avatar Apr 09 '25 15:04 sneridagh

@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.

wesleybl avatar Apr 09 '25 15:04 wesleybl

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

davisagli avatar Jun 27 '25 21:06 davisagli

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?

wesleybl avatar Jun 30 '25 20:06 wesleybl

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++

wesleybl avatar Jun 30 '25 20:06 wesleybl

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 avatar Aug 27 '25 03:08 wesleybl

@wesleybl Thanks for the reminder. I'll try to find time for it soon.

davisagli avatar Aug 28 '25 04:08 davisagli

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/....

mamico avatar Sep 10 '25 10:09 mamico

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 avatar Sep 10 '25 10:09 mamico

@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.

davisagli avatar Oct 14 '25 10:10 davisagli

@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

mamico avatar Oct 14 '25 11:10 mamico

A last one, do we need a special documentation note on production deployments?

sneridagh avatar Oct 20 '25 20:10 sneridagh

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.

wesleybl avatar Oct 20 '25 21:10 wesleybl

@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.

davisagli avatar Oct 20 '25 21:10 davisagli

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.

wesleybl avatar Oct 20 '25 22:10 wesleybl

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 avatar Oct 20 '25 22:10 davisagli

@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.

davisagli avatar Oct 20 '25 22:10 davisagli

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 avatar Oct 21 '25 02:10 wesleybl

@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.

davisagli avatar Oct 21 '25 02:10 davisagli

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.

sneridagh avatar Oct 21 '25 06:10 sneridagh

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)

davisagli avatar Oct 21 '25 06:10 davisagli

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?

stevepiercy avatar Oct 21 '25 12:10 stevepiercy

@davisagli Thank you for taking it forward. I took care of renaming things as per @stevepiercy's suggestions. Let's go go go...πŸŽ‰

nileshgulia1 avatar Oct 21 '25 16:10 nileshgulia1

@nileshgulia1 Thank you, I really appreciate it. I'll check it later today.

davisagli avatar Oct 21 '25 16:10 davisagli

@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.

stevepiercy avatar Oct 22 '25 08:10 stevepiercy

@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.

stevepiercy avatar Oct 22 '25 08:10 stevepiercy