[ADMIN] fix path concat to support admin serving at / path

[dekinderfiets]

What does it do?

Fixed the concatenation of paths.
When strapi.config.admin.path value is / (According to the docs, it should be supported), koa-static serving throws an AssertionError since the path is cannot be registered because the registered route is//:path* instead of /:path*.

Why is it needed?

It is needed so we can support serving the admin at https://example.com/ and not only at https://example.com/something

How to test it?

To reproduce the error, just set the url key at config/admin(.ts/.js) to / and start Strapi.

Related issue(s)/PR(s)

A few days ago someone faced the same issue at the forums:
https://forum.strapi.io/t/routing-for-separate-subdomains-for-api-and-admin-page/29177

[y2-shayb]

👌

[alexiuscrow]

This PR looks really great. I think this would greatly improve path routing.
Thanks, @dekinderfiets!

[vercel]

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

[derrickmehaffy]

I have a feeling this might break a few things in recent updates. @dekinderfiets have you tested this in a deployed environment with something like nginx and no-prefix base path? /

[dekinderfiets]

@derrickmehaffy

I have a feeling this might break a few things in recent updates. @dekinderfiets have you tested this in a deployed environment with something like nginx and no-prefix base path? /

I tried it all the variations mentioned above:

strapi.config.admin.path value	result
""	"/:path*"
"/"	"/:path*"
"/foo"	"/foo/:path*"
"/foo/"	"/foo/:path*"
"foo/"	"/foo/:path*"
"foo"	"/foo/:path*"

with an Nginx instance (Kubernetes Ingress) and it worked.
If you want a more stable and maintained solution for concatenating the url parts we can use url-join.
What do you think?

[derrickmehaffy]

@derrickmehaffy

I have a feeling this might break a few things in recent updates. @dekinderfiets have you tested this in a deployed environment with something like nginx and no-prefix base path? /

I tried it all the variations mentioned above:

strapi.config.admin.path value result
"" "/:path*"
"/" "/:path*"
"/foo" "/foo/:path*"
"/foo/" "/foo/:path*"
"foo/" "/foo/:path*"
"foo" "/foo/:path*"
with an Nginx instance (Kubernetes Ingress) and it worked. If you want a more stable and maintained solution for concatenating the url parts we can use url-join. What do you think?

Ah I don't think we need that package, I was more so worried about the various nginx proxy (or generic proxy application) handling.

[dekinderfiets]

@derrickmehaffy Ok, so what else we need to merge this PR? 😃

[derrickmehaffy]

Need to get engineers approval, I'll mention it to them in our internal slack

[derrickmehaffy]

Experimental release in progress for testing: https://github.com/strapi/strapi/actions/runs/6224864249

[derrickmehaffy]

Experimental version for testing: 0.0.0-experimental.e85c31c67a29c380cea20e131f8c2b8c4e4fbe82

[innerdvations]

This looks good for me and seemed to work fine, but since it's part of the core path routing if someone else can do some more thorough QA to double-check me to make sure we don't break anything as a side effect ( @derrickmehaffy do you have time?) that would be great, then we can merge 🚀

[derrickmehaffy]

Yeah I'll do some of my prod type testing with Nginx and caddy to see how well it works in a real world use-case.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
contributor-docs	❌ Failed (Inspect)			May 16, 2024 2:36pm

[dekinderfiets]

Hi @derrickmehaffy, could you please let me know if you had a chance to check if the PR works in a real world use-case? Thank you!