Skip to content

Enable local network sharing#1284

Merged
mattstauffer merged 2 commits intolaravel:masterfrom
thinkverse:fix-local-network-sharing
Feb 1, 2023
Merged

Enable local network sharing#1284
mattstauffer merged 2 commits intolaravel:masterfrom
thinkverse:fix-local-network-sharing

Conversation

@thinkverse
Copy link
Copy Markdown
Contributor

This PR enables a rudimentary version of local network sharing with the fix mentioned in (#440 (comment)). This pull request was suggested in (laravel/docs#8267) by @driesvints.

I've tested the fix locally and it does enable local network sharing, though it's rudimentary at best since assets don't load properly given the URL doesn't match.

For example, if I have http://laravel.test and access is via local network sharing http://192.168.1.111/laravel.test, the site does load. But the asset URLs will use http://laravel.test and thus not load. I'm not sure if that is due to the @vite directive or something else. The same thing happens with the asset helper, which might be because of the APP_URL.

I did manage to get the CSS, JavaScript, and HMR working with Vite by starting the Vite server using the local IP address passed in as the host.

npm run dev -- --host 192.168.1.111

Since Vite adds the IP address in the script and link files, the IP address used for the host must be an address that can be accessed by all devices over the network, The safest route is to use the same IP address of the device you're trying to expose.

Not sure how to go about fixing the issue of built assets not loading due to the URL not matching though. 🤷‍♂️

By removing te IP address from the HTTP_HOST we enable
a rudimentary version of local network sharing.

Co-authored-by: Mickael Urrutia <mickael.urrutia@utc.fr>
@thinkverse
Copy link
Copy Markdown
Contributor Author

Also, could anyone confirm or deny if this breaks Laravel Valets' wildcard DNS support? I don't use x|nip.io services so sadly cannot check that myself.

@mattstauffer
Copy link
Copy Markdown
Collaborator

I'm good with this modification, but I'll need someone who uses nip.io to test it out for me first.

Put out a request: https://twitter.com/stauffermatt/status/1598332500056621062

@F0rsaken
Copy link
Copy Markdown

F0rsaken commented Dec 7, 2022

I tested it on my setup and it worked, but there is a problem with Drivers. BasicValetDriver has a line

$_SERVER['SERVER_ADDR'] = '127.0.0.1';

that overrides server address, making a redirect to http://site-name.test. I don't know why there is a 127.0.0.1 override, but we can make fallback like this, where we can replace the ovveride condition with a proper one:

$serverAddr = $_SERVER['SERVER_ADDR'];

// condition when to override the server address if necessary
if (!$serverAddr || $serverAddr == 'localhost') {
    $_SERVER['SERVER_ADDR'] = '127.0.0.1';
}

same goes for every driver that do this. I can make a PR, but I would need to know when to enable this override 😄

@mattstauffer
Copy link
Copy Markdown
Collaborator

@F0rsaken Unfortunately I don't know yet whether your code there would or wouldn't be enough... I'm guessing it would be? I'll test it out and see if I can figure out.

@mattstauffer
Copy link
Copy Markdown
Collaborator

OK. I made a modification similar to yours @F0rsaken, but I didn't worry about the localhost check.

In theory that means this PR should work. I'll test it out now or shortly.

@mattstauffer
Copy link
Copy Markdown
Collaborator

Well.. this isn't perfect, but it at least is a start, with a bit of a potential fix, and it makes the docs at least a bit less inaccurate. lol. I just ran it and it worked, but with the same issues mentioned w/r/t the assets.

Thanks @thinkverse!

@mattstauffer mattstauffer merged commit 0879fb8 into laravel:master Feb 1, 2023
@mattstauffer mattstauffer mentioned this pull request Feb 1, 2023
@thinkverse thinkverse deleted the fix-local-network-sharing branch February 2, 2023 18:52
mattstauffer added a commit that referenced this pull request Feb 8, 2023
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.

3 participants