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

[5.5] Supporting Laravel-Mix PR #1246, HMR w/ different Hosts and Ports #22826

Merged
merged 2 commits into from
Jan 18, 2018
Merged

Conversation

sifex
Copy link
Contributor

@sifex sifex commented Jan 16, 2018

Part One: #21545

This PR changes the 'mix' helper function to take URLs coming from the hot file. This feature should be fully backwards compatible with older Laravel Mix & Laravel installs.

Supporting Laravel-Mix Pull Request laravel-mix/laravel-mix#1246, allowing Hot Module Reloading with custom hosts and ports, allowing hot-module reloading across networks.

Please don't merge straight away, I'm awaiting a reply to laravel-mix/laravel-mix#1246

JefferyWay merged laravel-mix/laravel-mix#1246. Should be good to go

Let me know if you require tests for this code change,

Edit: Just realised that there's no other use of Str:: in the class. Let me know if you want me to use the native php string functions.

sifex added 2 commits October 6, 2017 01:19
This PR changes the 'mix' helper function to take URLs coming from the hot file. This feature is fully backwards compatible.

Supporting Laravel-Mix Pull Request #1246, allowing Hot Module Reloading with custom hosts and ports

Do not merge straight away, awaiting reply of laravel-mix/laravel-mix#1246

Let me know if you require tests
@taylorotwell
Copy link
Member

Will let @JeffreyWay chime in on this.

@taylorotwell
Copy link
Member

Can you explain with this does in kinda simple terms? Sorry, I'm not super up to speed on the latest front-end stuff. 😄

@@ -555,6 +555,12 @@ function mix($path, $manifestDirectory = '')
}

if (file_exists(public_path($manifestDirectory.'/hot'))) {
$url = file_get_contents(public_path($manifestDirectory.'/hot'));

if (Str::startsWith($url, 'http://') || Str::startsWith($url, 'https://')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can be shortened to:

if (Str::startsWith($url, ['http://', 'https://'])) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find

@JeffreyWay
Copy link
Contributor

Yep, this is fully backward compatible and fine to merge.

@taylorotwell taylorotwell merged commit e603618 into laravel:5.5 Jan 18, 2018
pelletiermaxime referenced this pull request in laravel-mix/laravel-mix Mar 3, 2018
Once Laravel is updated to reflect the change, we can add this back in.
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.

4 participants