Prefix path router #3592

giuliaghisini · 2022-08-29T13:11:16Z

Enable to publish Volto site under a prefixPath, for example
www.mymainsite.com/prefixPath

netlify · 2022-08-29T13:11:39Z

❌ Deploy Preview for volto failed. Why did it fail? →

Name	Link
🔨 Latest commit	`a38dda0`
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/6645ebd8e199fb0008479210

cypress · 2022-08-29T13:48:20Z

Passing run #7234 ↗︎

0	14	0	0	0

Details:

refactor: prettify components
Project: Volto	Commit: `ef6216a98a`
Status: Passed	Duration: 02:07 💡
Started: Nov 28, 2023 11:05 AM	Ended: Nov 28, 2023 11:07 AM

Review all test suite changes for PR #3592 ↗︎

src/helpers/Api/APIResourceWithAuth.js

src/helpers/Api/Api.js

src/components/theme/View/FileView.jsx

src/components/theme/View/ImageView.jsx

pnicolli · 2022-09-05T07:42:45Z

Link view now working, Image view still has issues with the download link which is not working (see this example)

sneridagh · 2022-09-15T10:59:35Z

During the Volto Team meeting we agreed that we would have to put in place a whole round of Cypress tests pointing to a deployment using this. I can imagine that in the future will be quite easy to break the whole feature if one does not have in mind it.

I think that could be time consuming, but might not have much difficulty. What do you think?

sneridagh · 2022-10-03T08:36:46Z

@mamico @giuliaghisini could you share the reverse proxy config you have on such deployments?

sneridagh · 2022-10-03T08:38:08Z

We should add some documentation about how to setup such a deployment.
Also, I'll try to write a traefik config for testing it properly in CI.

cekk · 2022-10-03T09:27:43Z

@sneridagh our deployment config is a bit complicated because there are several urls and frontend names, and it's made in varnish and not in nginx/apache.

The conf itself for using this branch is easy. Here is an example for nginx:

upstream backend {
  server host.docker.internal:8080;
}
upstream frontend {
  server host.docker.internal:3000;
}

server {
  listen 80  default_server;
  server_name  localhost;

  location ~ /foo/\+\+api\+\+($|/) {
      rewrite ^/foo/\+\+api\+\+($|/.*) /VirtualHostBase/http/${server_name}/Plone/++api++/VirtualHostRoot/_vh_foo$1 break;
      proxy_pass http://backend;
  }


  location ~ /foo($|/) {
    
    # the standard nginx conf
    ...

    proxy_pass http://frontend;
  }
}

tiberiuichim · 2022-10-03T09:31:38Z

@cekk One other thing is important to document, the prefix path /foo corresponds to the Plone root, not a /foo subfolder in Plone.

cekk · 2022-10-03T09:49:49Z

Yes, /foo points to the root of Plone site

sneridagh · 2022-10-04T07:16:41Z

@cekk To make it work I had to launch Volto with:

RAZZLE_PREFIX_PATH=/foo RAZZLE_API_PATH=http://localhost/foo yarn start

I would have expect that given a RAZZLE_PREFIX_PATH, the other would have adjusted automatically (as seamless mode promises). I am doing something wrong? because given a look at the code, it seems it should, right?

sneridagh · 2022-10-04T07:20:31Z

@cekk Forget the question, I'm still asleep. 😅

sneridagh · 2022-10-04T09:52:19Z

Added tentative tests: #3719 see comments.

packages/volto-slate/src/editor/plugins/StyleMenu/index.js

nileshgulia1 · 2022-12-09T13:47:47Z

I would like to take this forward. Locally looks good. I will checkout this branch on one of our projects first and see if I find some issues.

State of this PR:

This PR is based on basename react-router prop which adds an initial entry to browser "history" stack and allows all links to be prefixed with basename by react-router. The static assets like images need to be explicitly prefixed. I amended an express-middleware which re-directs to app base path on initial load.

There is another approach in this PR, which uses a store enhancer middleware to prefix all the router paths(amending history accordingly) and modified flattenToAppUrl method.

What's left are the cypress tests #3719 which should also account for prefix path in the URLs. I will try to have a look into them.

I personally like the basename approach and let react-router handle the prefixes. However, we need to think about the non-router links and static assets. What do you think @sneridagh @davisagli @tiberiuichim @pnicolli ?

wesleybl · 2023-07-03T20:51:25Z

@giuliaghisini @nileshgulia1 @sneridagh any chance we have this feature in master?

wesleybl · 2023-07-04T13:05:51Z

I updated the branch and run:

RAZZLE_PREFIX_PATH=/my-prefix yarn start

Then I got the error:

ValidationError: Invalid options object. Dev Server has been initialized using an options object that does not match the API schema.
 - options has an unknown property 'publicPath'. These properties are valid:
   object { allowedHosts?, bonjour?, client?, compress?, devMiddleware?, headers?, historyApiFallback?, host?, hot?, http2?, https?, ipc?, liveReload?, magicHtml?, onAfterSetupMiddleware?, onBeforeSetupMiddleware?, onListening?, open?, port?, proxy?, server?, setupExitSignals?, setupMiddlewares?, static?, watchFiles?, webSocketServer? }
    at validate (/home/user/git/volto/node_modules/webpack-dev-server/node_modules/schema-utils/dist/validate.js:115:11)
    at new Server (/home/user/git/volto/node_modules/webpack-dev-server/lib/Server.js:231:5)
    at new razzleDevServer (/home/user/git/volto/node_modules/razzle/config/razzleDevServer.js:10:5)
    at /home/user/git/volto/node_modules/razzle/scripts/start.js:181:33
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

I'll take a look at it.

tiberiuichim · 2023-07-04T18:27:25Z

@wesleybl it's due to the changes in razzle.config.js

wesleybl · 2023-07-04T19:18:42Z

@tiberiuichim I fixed this in: f011df6

src/helpers/Url/Url.js

command

wesleybl · 2024-03-18T16:13:19Z

@sneridagh @nileshgulia1 add core test with prefixed path. Can you take a look please? Can we merge this PR?

sneridagh · 2024-03-31T15:27:52Z

docs/source/deploying/prefixed-root.md

+
+For a production setup, when hosting Volto behind a proxy HTTP server, you can configure your rewrite rules to something like the following, in this case for Apache.
+
+```apache


The recommended deployment setup is using Docker. I know that the documentation about this is in a sorry state, but we cannot add more confusion about this by dropping an Apache config here. I don't have a good solution for this one :(

In the upcoming sprints we should work on documenting properly the deployment story.

/cc @ericof @davisagli @stevepiercy @fredvd

@sneridagh would it be an idea to use an example with nginx, since docker uses nginx?

sneridagh · 2024-03-31T15:28:41Z

docs/source/deploying/prefixed-root.md

+RAZZLE_PREFIX_PATH=/level1/level2 RAZZLE_API_PATH=http://example.com/level1/level2 yarn start
+```
+
+Note, as you'll be integrating Volto with an existing website, you need to configure `config.settings.externalRoutes` so that the router knows which routes it should consider internal.


Need more elaboration and reasoning why this is necessary.

sneridagh · 2024-03-31T15:29:15Z

docs/source/deploying/prefixed-root.md

+```
+
+The prefix location will be used regardless of how you start Volto, whether in development or production mode.
+When developing, though, if your backend is something other then `http://localhost:8080`, you'll need to provide your own solution for how to handle things.


So in development I don't have to set it up and act as I'm in the root then?

sneridagh · 2024-03-31T15:40:42Z

packages/volto/cypress/tests/core/basic/actions.js

@@ -44,7 +46,7 @@ describe('actions Tests', () => {
      cy.get('a[class="icon-align-name"]').should(
        'have.attr',
        'href',
-        '/my-page-1/contents',
+        `${prefixPath}/my-page-1/contents`,


Can't we do it more elegant by invoking the baseUrl from Cypress config?

@sneridagh yes would be. And it would be less code to change. When I have time I will change this.

@sneridagh I started looking into this but that won't be possible. baseURL contains the URL with the domain. For example:

http://localhost/foo

But links in HTML are relative. For example:

/foo/my-document

sneridagh · 2024-03-31T15:41:36Z

packages/volto/cypress/support/commands.js

@@ -837,38 +842,14 @@ Cypress.Commands.add('clickSlateButton', (button, timeout = 1000) => {
  }).click({ force: true }); // force click is needed to ensure the button in visible in view.
 });

-// Helper functions


+1 I started this already in a recent PR. A merge might be needed.

sneridagh · 2024-03-31T15:42:45Z

packages/volto/razzle.config.js

+    if (target === 'web' && dev) {
+      config.devServer.devMiddleware.publicPath = prefixPath;
+    }
+    const pp = config.output.publicPath;


No cryptic vars, please.

And regarding the dev server, is it necessary to also use the prefix while developing?

In webpack 5, the public path in dev mode needs to be prefixed as well. http://localhost:3001/foo/static/js/client.js

sneridagh · 2024-03-31T15:46:09Z

packages/volto/src/components/manage/Controlpanels/Controlpanels.jsx

@@ -130,64 +136,66 @@ function Controlpanels({
    : [];
  const { filterControlPanels } = config.settings;

-  const filteredControlPanels = map(


I don't get the reason for this. Which is the rationale?

might not need it after 9a6b25f ?
-cc @wesleybl

In 3ad97df, @nileshgulia1 performed a validation to see if there was a controlpanel. I kept that validation. See below:

https://github.com/plone/volto/pull/3592/files#diff-10e8b3997f0087a776219f684dfa7c6440933357dee431c5614d6d4289191c00R139

This prevents an on-screen error if the backend does not return the controlpanels.

sneridagh · 2024-03-31T15:48:26Z

packages/volto/src/components/manage/UniversalLink/UniversalLink.jsx

  flattenToAppURL,
  isInternalURL,
  URLUtils,
-} from '@plone/volto/helpers/Url/Url';


We decided not using barrel indexes in Volto anymore, so going back here is not an option.

sneridagh · 2024-03-31T15:50:04Z

packages/volto/src/components/manage/UniversalLink/UniversalLink.jsx

@@ -109,7 +110,7 @@ const UniversalLink = ({
  } else if (isDownload) {
    tag = (
      <a
-        href={flattenToAppURL(url)}
+        href={addPrefixPath(flattenToAppURL(url))}


So what if instead of modifying any invocation to flattenToAppURL (and I'm sure we are missing a lot), we incorporate this to it? I think it's not acceptable to modify every single apparition of it and the future ones.

@sneridagh I encountered challenges when attempting to integrate it directly into the "flattenToAppUrl" function, primarily due to the occurrence of duplicate prefixes because we already have a router basename set in this PR. Just to let you know we have recently deployed https://water.europa.eu/freshwater which is using bits and pieces of this PR. We did some customizations which will be removed when volto 17 is integrated into that site. I'm not sure if we applied special deployment tactics maybe @olimpiurob or @claudiaifrim knows more about this context.

Just for people who wants to test this feature, currently for development we use

RAZZLE_PREFIX_PATH=/freshwater RAZZLE_INTERNAL_API_PATH=http://x.x.x.x:port/Plone yarn start

You might also need to set RAZZLE_API_PATH if your backend lives in another site than /Plone.

@nileshgulia1 I didn't understand the difficulty of using addPrefixPath inside flattenToAppURL. Are there situations where it is not necessary to use addPrefixPath?

@wesleybl Basically paths that are rendered using <Link /> or <UniversalLink/> need not be prefixed. This also includes hashlinks. In volto-prefixpath we don't set the router basename as prefix, thus we use this approach.
The image components in our addons were our major source of trouble because in volto 17 we have a single Image component and the rule is to use it everywhere to render images. So, we use this approach until we migrate all addons to volto 17.

sneridagh

@plone/volto-team We need to talk about this in our next meeting.

Finally I took a long look at it, and TBH I have lots of doubts. Sorry to be that pessimistic, but it's my sincere opinion.

I understand that this is a long requested feature. However, implement this if our Router does not support it OOTB (which is what it's happening) seems to me a lot. A lot of code to maintain, a lot of pitfalls that lie ahead if we adopt this now.

Take the example of Remix. They just implemented this, and baked it into latest RR and their Vite build this last month. They did it by using a global baseName support: https://remix.run/blog/remix-vite-stable#basename-support

I can tell you this, complicating Volto codebase will only make harder future refactors of its fundamental components. I don't want this to happen. I'm also convinced that make the current tests pass is not enough. I'm sure we have lots of edge cases uncovered.

This is intrusive enough for it to have its own major release alone, and I won't include it in 18... It has enough changes in there, and it's important enough already, being the targeted Plone 6.1 release, and we are already at the end of the alpha phase.

Let's discuss it in the next meeting, let's see if we can polish rough edges. I'd love that everyone involved and interested be there to talk about it.

wesleybl · 2024-04-01T15:39:24Z

This is intrusive enough for it to have its own major release alone, and I won't include it in 18...

@sneridagh so would this be for Volto 19? Any predictions on when Volto 19 will start?

stevepiercy · 2024-04-01T22:02:49Z

Any predictions on when Volto 19 will start?

It already has, by virtue of this pull request. I think this is the second feature designated for Volto 19, after my PLIP for timezone, date, and time support. I think it would be good to create a PLIP for discussion and add it to the Volto Roadmap GitHub project. I also created a 19.x.x milestone to start tagging issues and pull requests.

stevepiercy

I've put this item on the agenda for the next Volto Team Meeting to discuss documentation, implementation details, targeting a Volto release, and possibly other items that come up.

stevepiercy · 2024-04-03T20:42:59Z

docs/source/deploying/prefixed-root.md

I think that this page needs to have better narrative structure, such as the following:

Introduction that states the essential and basic requirements for a deployment, regardless of root or non-root. This might already exist somewhere in the Deploying section. Side note: that entire section needs to be cleaned up, and I think the index page should have this introduction as its introduction, so we can easily refer to it. We can also use an include to avoid copy-paste drift.

Clarify what these requirements do and why they are necessary.

Then add to the basic requirements those things that are required for a non-root deployment.

Again, clarify what these requirements do and why they are necessary.

Finally show examples of implementation details, whether it is Apache, nginx, or whatever. It is OK to have multiple examples. In fact I would use Sphinx Design with a tabbed code example interface.

nileshgulia1 · 2024-04-09T18:46:39Z

Minutes from the @plone/volto-team meeting:

It was suggested that in dev mode, the prefix should not be a concern. Instead, configurations should be adjusted to exclusively utilize prefix during deployment.
Explore the possibility to integrate the functionalities of addprefixPath into flattenToAppUrl covering all the edge cases we might have. react-router v6 has basename support. We need to further investigate how this feature is intended to function, both with and without the React Router upgrade( we are still using v5). The current implementation may compel future developers to rely extensively on this additional helper. Also what about the existing addons? they will have to be migrated as well.
The current cypress testing strategy needs a rehaul according to point 2. It looks unsustainable to incorporate addPrefixPath across all forthcoming tests. Perhaps there is a way to run tests just using a cypress baseUrl? What about the internalUrls those need to be prefixed like Images, links?
In the context of deployment documentation, we should explicitly outline if we need to use apache rules or docker.

giuliaghisini requested review from tiberiuichim, sneridagh and cekk August 29, 2022 13:11

mamico requested changes Aug 29, 2022

View reviewed changes

src/helpers/Api/APIResourceWithAuth.js Outdated Show resolved Hide resolved

src/helpers/Api/Api.js Outdated Show resolved Hide resolved

giuliaghisini requested a review from mamico August 30, 2022 13:10

giuliaghisini commented Sep 2, 2022

View reviewed changes

src/components/theme/View/FileView.jsx Outdated Show resolved Hide resolved

src/components/theme/View/ImageView.jsx Outdated Show resolved Hide resolved

mamico approved these changes Sep 12, 2022

View reviewed changes

mamico self-requested a review September 12, 2022 07:11

sneridagh added this to the 16.x.x milestone Oct 1, 2022

nileshgulia1 reviewed Dec 8, 2022

View reviewed changes

packages/volto-slate/src/editor/plugins/StyleMenu/index.js Outdated Show resolved Hide resolved

nileshgulia1 mentioned this pull request Jan 21, 2023

Prefix path setup #4290

Open

9 tasks

sneridagh removed this from the 16.x.x milestone Jun 6, 2023

wesleybl reviewed Jul 6, 2023

View reviewed changes

src/helpers/Url/Url.js Outdated Show resolved Hide resolved

wesleybl added 7 commits March 15, 2024 13:13

Test core control panels with prefixed site

8cf3b05

Test core blocks with prefixed site

5ce2e94

Add prefix to the URL of Image Gallery images

a59a704

Pass the prefixPath environment variable to the test-acceptance-prefixed

2944946

command

Test core blocks listing with prefixed site

235e4c5

Test core volto slate with prefixed site

38e428c

Merge branch 'main' into prefix_path_router

1e2620e

sneridagh reviewed Mar 31, 2024

View reviewed changes

sneridagh requested changes Mar 31, 2024

View reviewed changes

stevepiercy modified the milestone: 18.x.x Mar 31, 2024

nileshgulia1 added the 32 needs: review label Apr 3, 2024

stevepiercy reviewed Apr 3, 2024

View reviewed changes

nileshgulia1 added 5 commits May 13, 2024 16:55

Merge branch 'main' into prefix_path_router

f3a51a4

fix: use prefix in flattenToAppUrl, keep assets intact in dev

433ad1c

remove: addprefixPath helper

dd553b8

fix: client and server warnings

d66b0ac

Merge branch 'main' into prefix_path_router

a38dda0


		For a production setup, when hosting Volto behind a proxy HTTP server, you can configure your rewrite rules to something like the following, in this case for Apache.

		```apache

Prefix path router #3592

Are you sure you want to change the base?

Prefix path router #3592

Conversation

giuliaghisini commented Aug 29, 2022

netlify bot commented Aug 29, 2022 • edited

❌ Deploy Preview for volto failed. Why did it fail? →

cypress bot commented Aug 29, 2022 • edited

Passing run #7234 ↗︎

Review all test suite changes for PR #3592 ↗︎

pnicolli commented Sep 5, 2022 • edited by mamico

sneridagh commented Sep 15, 2022 • edited

sneridagh commented Oct 3, 2022

sneridagh commented Oct 3, 2022

cekk commented Oct 3, 2022

tiberiuichim commented Oct 3, 2022

cekk commented Oct 3, 2022

sneridagh commented Oct 4, 2022

sneridagh commented Oct 4, 2022 • edited

sneridagh commented Oct 4, 2022

nileshgulia1 commented Dec 9, 2022

State of this PR:

wesleybl commented Jul 3, 2023

wesleybl commented Jul 4, 2023 • edited

tiberiuichim commented Jul 4, 2023

wesleybl commented Jul 4, 2023

wesleybl commented Mar 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sneridagh left a comment

Choose a reason for hiding this comment

wesleybl commented Apr 1, 2024

stevepiercy commented Apr 1, 2024

stevepiercy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nileshgulia1 commented Apr 9, 2024

netlify bot commented Aug 29, 2022 •

edited

cypress bot commented Aug 29, 2022 •

edited

pnicolli commented Sep 5, 2022 •

edited by mamico

sneridagh commented Sep 15, 2022 •

edited

sneridagh commented Oct 4, 2022 •

edited

wesleybl commented Jul 4, 2023 •

edited