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 missing port value across subdomains #293

Merged
merged 3 commits into from Jun 5, 2020

Conversation

bakerkretzmar
Copy link
Collaborator

@bakerkretzmar bakerkretzmar commented May 22, 2020

This PR fixes #208 by ensuring that the port for the current route/request is always appended when using Ziggy's route() function, even when building a route for a different domain.

Serving an app on port 8000 and the domain ziggy.test, and given the following routes:

Route::view('/', 'home')->name('home');

Route::domain('foo.ziggy.test')->group(function () {
    Route::get('user/{id}', function () {
        return view('user');
    })->name('users');
});

In Laravel, we get this:

route('home');
// http://ziggy.test:8000

route('users', 1);
// http://foo.ziggy.test:8000/user/1

In a browser console using Ziggy, previously, we would get this:

route('home').url();
// http://ziggy.test:8000/

route('users', 1).url();
// http://foo.ziggy.test/user/1 (missing port)

This PR makes Ziggy's output match Laravel's, so that instead its output looks like this:

route('users', 1).url();
/// http://foo.ziggy.test:8000/user/1

The change that caused this issue was introduced in #75, which added support for ports. That PR and the associated tests explicitly ensured that the port was not added unless the domain of the route being generated matched the current domain exactly. It's not clear to me why this functionality was set up that way, and after some research and testing I'm pretty sure it should actually work the way this PR demonstrates, so I updated that test as well to basically do the opposite of what it did before.

This is likely a breaking change and should not be released before v1.

See #75, #74, and laravel/framework#26247.

@bakerkretzmar bakerkretzmar added this to the v1.0 milestone May 22, 2020
@bakerkretzmar bakerkretzmar merged commit 13e4740 into master Jun 5, 2020
@bakerkretzmar bakerkretzmar deleted the jbk/cross-domain-port branch June 5, 2020 15:16
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.

Missing port value when using different domains in RouteServiceProvider
1 participant