Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: typo for cleanUrls: true on Vercel #12

Merged
merged 4 commits into from
Aug 4, 2023

Conversation

styfle
Copy link
Contributor

@styfle styfle commented Aug 2, 2023

I tested this this by deploying with { "cleanUrls": true }. I think this was just a typo in the readme.

@vercel
Copy link

vercel bot commented Aug 2, 2023

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

Name Status Preview Comments Updated (UTC)
vercel-cleanurls-false-trailingslash-false ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am
vercel-cleanurls-false-trailingslash-true ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am
vercel-cleanurls-false-trailingslash-undefined ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am
vercel-cleanurls-true-trailingslash-false ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am
vercel-cleanurls-true-trailingslash-true ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am
vercel-cleanurls-true-trailingslash-undefined ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am
vercel-default ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2023 9:13am

@vercel
Copy link

vercel bot commented Aug 2, 2023

@styfle is attempting to deploy a commit to the Trailing Slash Team Team on Vercel.

A member of the Team first needs to authorize it.

@slorber
Copy link
Owner

slorber commented Aug 3, 2023

Thanks

The results on my existiung test deployment are different surprisingly:
https://vercel-cleanurls-true-trailingslash-undefined.vercel.app/

CleanShot 2023-08-03 at 17 42 06@2x

I'm not sure how generated those deployments anymore, maybe I did a typo in the config while deploying this deployment in particular. Is there any way on your side to check the config I used on my deployment?

If that helps: https://vercel.com/trailing-slash-team/vercel-cleanurls-true-trailingslash-undefined

I don't see any vercel.json on my source, so maybe I used the UI settings? Is this still possible (or has ever been?) to set this config through the UI settings tab?

@slorber
Copy link
Owner

slorber commented Aug 3, 2023

Ohhh I remember now: I used one branch per vercel.json config file.

The appropriate branch: https://github.com/slorber/trailing-slash-guide/blob/vercel-cleanurls-true-trailingslash-undefined/static/vercel.json

It looks wired correctly:

CleanShot 2023-08-03 at 17 48 31@2x

Note I explicitly use trailingSlash: undefined, can it have any impact?


This is really weird, my deployment shows a preview screenshot that looks like your results, but it's not what I see in practice after navigating to this deployment locally!

https://vercel.com/trailing-slash-team/vercel-cleanurls-true-trailingslash-undefined shows:

CleanShot 2023-08-03 at 17 50 50@2x

And this other url https://vercel.com/trailing-slash-team/vercel-cleanurls-true-trailingslash-undefined/3Xw7ZAyiKSbZwFawchhK9k1f8nAb shows:

CleanShot 2023-08-03 at 17 52 15@2x

Edit: damn, I refreshed the deployment URL https://vercel.com/trailing-slash-team/vercel-cleanurls-true-trailingslash-undefined and now the screenshot changed!

CleanShot 2023-08-03 at 17 53 28@2x

@styfle
Copy link
Contributor Author

styfle commented Aug 3, 2023

This looks like a typo too:

"trailingSlash": "undefined"

It has the string "undefined" instead of the actual value of undefined.

That might explain why that commit failed to deploy.

I created another PR with the fix:

@styfle
Copy link
Contributor Author

styfle commented Aug 3, 2023

@slorber Can you approve the deployments in both these PRs to confirm that the previews work as expected now?

@slorber
Copy link
Owner

slorber commented Aug 3, 2023

Ahhh 🤯 good catch!

Thanks, that now works!

https://vercel-cleanurls-true-trailingslash-undefined.vercel.app/

CleanShot 2023-08-03 at 19 02 40

Not exactly the results you updated in this PR: we still have some server redirects (expected IMHO)

Also: I updated both deployments with "undefined", so both might have a problem, not just this one.

Also, the table is displayed there too: https://github.com/slorber/trailing-slash-guide/blob/main/docs/Hosting-Providers.md


Normally the full Markdown table is automatically generated with code in the console if you go on the deployment index: https://vercel-cleanurls-true-trailingslash-undefined.vercel.app/

It seems they shut down the CORS proxy I was using: https://github.com/slorber/trailing-slash-guide/blob/main/static/index.html#L202

I can fix it so that we don't have to manually edit the Markdown table, but do you know a good free CORS proxy I could rely on?

@styfle
Copy link
Contributor Author

styfle commented Aug 3, 2023

do you know a good free CORS proxy I could rely on?

You could try this one: https://github.com/josmas/vercel-proxy

Demo: http://vercel-proxy.vercel.app

@slorber
Copy link
Owner

slorber commented Aug 3, 2023

Probably not the most robust CORS impl but it seems to work:

Will try to update it on this repo tomorrow 👍

@slorber
Copy link
Owner

slorber commented Aug 4, 2023

Table regeneration fixed 👍 thanks for the fix

@slorber slorber merged commit 1829643 into slorber:main Aug 4, 2023
6 of 7 checks passed
@styfle styfle deleted the fix-vercel-clean-urls branch August 4, 2023 14:23
@styfle
Copy link
Contributor Author

styfle commented Aug 4, 2023

@slorber What do you think about removing one of these rows since "Default" and (cleanUrls: false, trailingSlash: undefined) are both the same (since those are the default values)?

image

Docs: https://vercel.com/docs/concepts/projects/project-configuration

@slorber
Copy link
Owner

slorber commented Aug 10, 2023

👍 simplified/deduplicated for Vercel + Netlify

CleanShot 2023-08-10 at 18 43 43@2x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants