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(qwik-city): enforce / on base paths #6301

Merged
merged 1 commit into from
May 15, 2024
Merged

fix(qwik-city): enforce / on base paths #6301

merged 1 commit into from
May 15, 2024

Conversation

wmertens
Copy link
Member

@wmertens wmertens commented May 12, 2024

@mhevery not sure if this should go in only v2 because it might be slightly breaking.

Copy link

cloudflare-pages bot commented May 12, 2024

Deploying qwik-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 71c9b6c
Status: ✅  Deploy successful!
Preview URL: https://70dfe784.qwik-8nx.pages.dev
Branch Preview URL: https://base-error.qwik-8nx.pages.dev

View logs

@PatrickJS
Copy link
Member

I think we might need q:origin because I want the base path to be the same but different origin. and if I want to have a certain version of base then I'll change the base path to that version in ssr entry

@wmertens
Copy link
Member Author

@PatrickJS could you elaborate on origin? I don't understand the use case

@PatrickJS
Copy link
Member

PatrickJS commented May 14, 2024

so the base path might be the same but you may change the origin to an assets domain. in one of my apps we're using base to change the origin to an assets cdn while the app is hosted on another origin domain

if I use base to change the origin then it will start with https:// and not /

I think it's better to force base to only be the path and not the origin. and provide a better way to change the origin

Copy link

netlify bot commented May 15, 2024

Deploy Preview for qwik-insights ready!

Name Link
🔨 Latest commit 71c9b6c
🔍 Latest deploy log https://app.netlify.com/sites/qwik-insights/deploys/6644945e361ff60008348179
😎 Deploy Preview https://deploy-preview-6301--qwik-insights.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@wmertens
Copy link
Member Author

@PatrickJS I think you can achieve it without q:origin. Set vite base to the URL prefix of the app and set the renderToStream base to the full url of the origin+prefix+build/

@wmertens wmertens enabled auto-merge (squash) May 15, 2024 10:56
@wmertens wmertens merged commit b381b22 into main May 15, 2024
28 checks passed
@wmertens wmertens deleted the base-error branch May 15, 2024 11:09
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.

None yet

3 participants