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

Error in view data / url prop #359

Open
kimhf opened this issue Jan 27, 2022 · 18 comments · Fixed by #592
Open

Error in view data / url prop #359

kimhf opened this issue Jan 27, 2022 · 18 comments · Fixed by #592

Comments

@kimhf
Copy link

kimhf commented Jan 27, 2022

Currently if the project has the base url localhost/test and we navigate to some/sub/page the result is a url change to localhost/test/test/some/sub/page in the brower by inertiajs. This url no longer matches the route we are currently at.

The issue seems to be that a path as part of the base url is not handled correctly.

This line in \Inertia\Response causes the issue.

'url' => $request->getBaseUrl().$request->getRequestUri(),

@kimhf
Copy link
Author

kimhf commented Jan 27, 2022

I have created a pull request #360 with a possible solution to this issue.

@rhysemmerson
Copy link
Contributor

This looks like a duplicate of #236

@kimhf
Copy link
Author

kimhf commented Jan 31, 2022

I do not believe this is a duplicate of #236, but a solution might solve both issues.

@NickSdot
Copy link
Contributor

NickSdot commented Feb 5, 2022

@claudiodekker seems this was introduced in #333

If I am not mistaken this anyway shouldn't be Inertia's job. The behavior before was just fine and the issue which lead to the PR should be solvable by e.g. setting root in the NGINX config (untested).

@kimhf
Copy link
Author

kimhf commented Feb 6, 2022

It should in my opinion be Inertia's job to handle different server configurations. The problem as far as I can see is that #333 is the wrong fix for the problem. At least in my case where I'm setting RewriteBase /to/project in .htaccess

The combination of $request->getBaseUrl() + $request->getRequestUri() generates a invalid url with that server config. Looking at how fullUrl() / getUri() builds the url $request->getBaseUrl() + $request->getPathInfo() + $queryString (or fullUrl()) seems to be the right approach and should handle more server configurations.

It seems to me that $request->getBaseUrl() + $request->getRequestUri() are not intended to be combined together in that way.

@lotsofbytes
Copy link

Thanks @kimhf. I can't wait this gets released.

I have just started using inertia a couple days ago. This will solve my problem with a project in subfolder.

@w99910
Copy link

w99910 commented Feb 13, 2022

I hope this commit will be merged soon because it is seemed to be that this will solve my problem too.

@juancarlos-ctg
Copy link

Hope this gets fixed soon, it fixed my issue as well. ✊

@qbuache
Copy link

qbuache commented May 4, 2022

also interested in a fix, at the moment I have to mess with the toResponse function in /vendor/inertiajs/inertia-laravel/src/Response.php

@romanmartushev
Copy link

I am also interested in this fix! Currently unable to update a site to Laravel 9 because of this issue.

@dopesky
Copy link

dopesky commented May 12, 2022

This also fixed my issue. Hope it gets merged soon.

@lucraraujo
Copy link

Holy sh**! I lost one week of my life because of this bug!
I tested 3 different web server (apache, nginx, IIS) to finally find this bug!

@andrew1601
Copy link

I noticed this issue after submitting issue #421 and proposing PR #422

I spent 4 hours this afternoon before finding this bug!

also interested in a fix, at the moment I have to mess with the toResponse function in /vendor/inertiajs/inertia-laravel/src/Response.php

This is how I've dealt with it too in the meantime.

@Pachekoko
Copy link

This fixed my problem too, I'm glad I found the solution I was struggling since the beggining of this week. Thanks

@CristianSitov
Copy link

I spent a lot of time trying to fix this thing in my nginx conf, until I started looking at Laravel & InertiaJs packages to understand the source of the duplication. Definitely this patch is the right solution for the problem. I can't wait to see this merged (in the mean time, I'm going to production with a fork ¯_(ツ)_/¯).

@sinceow
Copy link

sinceow commented Sep 19, 2022

I can't wait to see this merged!!!!!!!!

@kentchang84
Copy link

Awaiting the merge, almost a year, gosh

@raghed-kahil
Copy link

raghed-kahil commented Nov 17, 2023

I have same issue so i downloaded the package and edited it localy.

Actually i also edited svelte package too.

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 a pull request may close this issue.