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

Conversation

@sifex
Copy link
Contributor

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 JeffreyWay/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 JeffreyWay/laravel-mix#1246

JefferyWay merged JeffreyWay/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 some commits Oct 5, 2017

Supporting Laravel-Mix PR #1246, HMR w/ different Hosts and Ports
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 JeffreyWay/laravel-mix#1246

Let me know if you require tests
@taylorotwell

This comment has been minimized.

Copy link
Member

commented Jan 17, 2018

Will let @JeffreyWay chime in on this.

@taylorotwell

This comment has been minimized.

Copy link
Member

commented Jan 17, 2018

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://')) {

This comment has been minimized.

Copy link
@ntzm

ntzm Jan 17, 2018

Contributor

This line can be shortened to:

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

This comment has been minimized.

Copy link
@sifex

sifex Jan 20, 2018

Author Contributor

Nice find

@JeffreyWay

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2018

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

@taylorotwell taylorotwell merged commit e603618 into laravel:5.5 Jan 18, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/styleci/pr The StyleCI analysis has failed - 5 files need addressing
Details

pelletiermaxime referenced this pull request in JeffreyWay/laravel-mix Mar 3, 2018

Temporarily remove.
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
Projects
None yet
4 participants
You can’t perform that action at this time.