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.4] Refactoring mix method (bis). #19666

Closed
wants to merge 12 commits into from
33 changes: 1 addition & 32 deletions src/Illuminate/Foundation/helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -522,41 +522,10 @@ function method_field($method)
* @param string $path
* @param string $manifestDirectory
* @return \Illuminate\Support\HtmlString
*
* @throws \Exception
*/
function mix($path, $manifestDirectory = '')
{
static $manifest;

if (! starts_with($path, '/')) {
$path = "/{$path}";
}

if ($manifestDirectory && ! starts_with($manifestDirectory, '/')) {
$manifestDirectory = "/{$manifestDirectory}";
}

if (file_exists(public_path($manifestDirectory.'/hot'))) {
return new HtmlString("//localhost:8080{$path}");
}

if (! $manifest) {
if (! file_exists($manifestPath = public_path($manifestDirectory.'/mix-manifest.json'))) {
throw new Exception('The Mix manifest does not exist.');
}

$manifest = json_decode(file_get_contents($manifestPath), true);
}

if (! array_key_exists($path, $manifest)) {
throw new Exception(
"Unable to locate Mix file: {$path}. Please check your ".
'webpack.mix.js output paths and try again.'
);
}

return new HtmlString($manifestDirectory.$manifest[$path]);
return app('mix')->mix($path, $manifestDirectory);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it looks strange to do (new Mix)->mix() why not something more like (new Mix)->create()..

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep or generate maybe?

Copy link
Contributor Author

@mathieutu mathieutu Jun 19, 2017

Choose a reason for hiding this comment

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

Yeah I thought about that.
create and generate aren't good ones either because you don't create or generate anything, you just find the good path in a file, so find? I didn't find something I liked, so I kept that because in any way, the method will always be called by the mix() helper, as usual.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about load?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could use resolve(). If Taylor merges it, he could always rename it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love resolve thank you! 🎉

}
}

Expand Down
271 changes: 271 additions & 0 deletions src/Illuminate/View/Mix/Mix.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,271 @@
<?php

namespace Illuminate\View\Mix;

use Illuminate\Support\Str;
use Illuminate\Support\HtmlString;

class Mix
{
/**
* The sanitized path to the asset.
*
* @var string
*/
protected $path;

/**
* The cached manifest.
*
* @var array
*/
protected $manifest;

/**
* The directory where the assets and the manifest file are.
*
* @var string
*/
protected $manifestDirectory;

/**
* The cache of mix state.
*
* @var bool
*/
protected $disabled = false;

/**
* The URI of HMR server.
*
* @var string
*/
protected $hmrURI = '//localhost:8080';

/**
* The name of file which prove that HMR is enabled.
*
* @var string
*/
protected $hmrFilename = '/hot';

/**
* The name of Mix Manifest file.
*
* @var string
*/
protected $manifestFilename = '/mix-manifest.json';

/**
* Get the path to a versioned Mix asset or a simple message if mix is disabled.
*
* @param string $path
* @param string $manifestDirectory
* @return \Illuminate\Support\HtmlString
*/
public function mix($path, $manifestDirectory = '')
{
if ($this->disabled) {
return $this->disabledPath();
}

return $this->getRealPath($path, $manifestDirectory);
}

/**
* Get the path to a versioned Mix file.
*
* @param string $path
* @param string $manifestDirectory
* @return \Illuminate\Support\HtmlString
*/
protected function getRealPath($path, $manifestDirectory)
{
$this->init($path, $manifestDirectory);

if ($this->hmrModeEnabled()) {
return $this->hmrPath();
}

return $this->compiledPath();
}

/**
* Set a sanitized version of assets.
*
* @param string $path
* @param string $manifestDirectory
* @return void
*/
protected function init($path, $manifestDirectory)
{
$this->path = $this->sanitize($path);
$this->manifestDirectory = $this->sanitize($manifestDirectory);
}

/**
* Get a sanitized version of a path.
*
* @param string $path
* @return string
*/
protected function sanitize($path)
{
if ($path && ! Str::startsWith($path, '/')) {
$path = "/{$path}";
}

return $path;
}

/**
* Check if the HRM mode of Mix is enabled.
*
* @return bool
*/
protected function hmrModeEnabled()
{
return file_exists(public_path($this->manifestDirectory.$this->hmrFilename));
}

/**
* Get the full path to the file through the HMR server.
*
* @return \Illuminate\Support\HtmlString
*/
protected function hmrPath()
{
return new HtmlString($this->hmrURI.$this->path);
}

/**
* Get the full path to the compiled file.
*
* @return \Illuminate\Support\HtmlString
*/
protected function compiledPath()
{
return new HtmlString($this->manifestDirectory.$this->getPathFromManifest());
}

/**
* Get a message instead of the path when mix is disabled.
*
* @return \Illuminate\Support\HtmlString
*/
protected function disabledPath()
{
return new HtmlString('Mix is disabled!');
}

/**
* Get the path from the manifest file.
*
* @return string
*
* @throws \Illuminate\View\Mix\MixException
*/
protected function getPathFromManifest()
{
if (! array_key_exists($this->path, $manifest = $this->getManifest())) {
throw new MixException(
"Unable to locate Mix file: {$this->path}. Please check your ".
'webpack.mix.js output paths and try again.'
);
}

return $manifest[$this->path];
}

/**
* Load the manifest file.
*
* @return array
*
* @throws \Illuminate\View\Mix\MixException
*/
protected function getManifest()
{
if (! $this->manifest) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to go further if $this->manifest is not empty. You can favor an early return:

if ($this->manifest) {
    return $this->manifest;
}

if (! file_exists($manifestPath = public_path($this->manifestDirectory.$this->manifestFilename))) {
    throw new MixException('The Mix manifest does not exist.');
}

$this->manifest = json_decode(file_get_contents($manifestPath), true);

if (json_last_error() === JSON_ERROR_NONE) {
    return $this->manifest;
}

throw new MixException("The Mix manifest isn't a proper json file.");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's your code right here, you made me change mine to yours for no valid reason, and now you're again changing your mind. It's just about a basic refactoring and you're commenting every line I'm writing for 3 days each time I make a new commit.

I have work to do, and I haven't not enough time to satisfy all your dreams on a basic function like this one. So, I don't know what's your goal, but please stop, or do it yourself!

Copy link
Contributor

@lucasmichot lucasmichot Jun 19, 2017

Choose a reason for hiding this comment

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

It's just about a basic refactoring

It's a great PR but a critical function.

if (! file_exists($manifestPath = public_path($this->manifestDirectory.$this->manifestFilename))) {
throw new MixException('The Mix manifest does not exist.');
}

$this->manifest = json_decode(file_get_contents($manifestPath), true);

if (json_last_error() !== JSON_ERROR_NONE) {
throw new MixException('The Mix manifest isn\'t a proper json file.');
}
}

return $this->manifest;
}

/**
* Disable the mix function (in case of tests for example).
*
* @param bool $disabled
* @return $this
*/
public function disable($disabled = true)
{
$this->disabled = $disabled;

return $this;
}

/**
* Enable the mix function (in case of it was disabled before).
*
* @param bool $enabled
* @return $this
*/
public function enable($enabled = true)
{
$this->disabled = ! $enabled;

return $this;
}

/**
* Set the URI of HMR sever.
*
* @param string $hmrURI
*
* @return $this
*/
public function setHmrURI($hmrURI)
{
$this->hmrURI = $hmrURI;

return $this;
}

/**
* Set the Mix Manifest filename.
*
* @param string $manifestFilename
*
* @return Mix
Copy link
Contributor

Choose a reason for hiding this comment

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

We use @return $this

*/
public function setManifestFilename($manifestFilename)
{
$this->manifestFilename = $this->sanitize($manifestFilename);

return $this;
}

/**
* Set the HMR hot filename.
*
* @param string $hmrFilename
*
* @return Mix
Copy link
Contributor

Choose a reason for hiding this comment

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

We use @return $this

*/
public function setHmrFilename($hmrFilename)
{
$this->hmrFilename = $this->sanitize($hmrFilename);

return $this;
}
}
10 changes: 10 additions & 0 deletions src/Illuminate/View/Mix/MixException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace Illuminate\View\Mix;

use Exception;

class MixException extends Exception
{
//
}
15 changes: 15 additions & 0 deletions src/Illuminate/View/ViewServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\View;

use Illuminate\View\Mix\Mix;
use Illuminate\View\Engines\PhpEngine;
use Illuminate\Support\ServiceProvider;
use Illuminate\View\Engines\FileEngine;
Expand All @@ -23,6 +24,8 @@ public function register()
$this->registerViewFinder();

$this->registerEngineResolver();

$this->registerMix();
}

/**
Expand Down Expand Up @@ -133,4 +136,16 @@ public function registerBladeEngine($resolver)
return new CompilerEngine($this->app['blade.compiler']);
});
}

/**
* Register the Mix instance.
*
* @return void
*/
public function registerMix()
{
$this->app->singleton('mix', function () {
return new Mix;
});
}
}
10 changes: 10 additions & 0 deletions tests/Foundation/FoundationHelpersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,14 @@ public function testUnversionedElixir()

unlink(public_path($file));
}

public function testMixMethod()
{
app()->instance(
'mix',
m::mock(\Illuminate\View\Mix\Mix::class)->shouldReceive('mix')->andReturn('bar')->getMock()
);

$this->assertSame('bar', mix('foo'));
}
}
Loading