Skip to content

Commit

Permalink
Update Includes helpers to return HtmlString objects
Browse files Browse the repository at this point in the history
Fixes #1734
  • Loading branch information
caendesilva committed Jun 26, 2024
1 parent 3a712b7 commit 4122038
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 39 deletions.
17 changes: 17 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ This serves two purposes:
- Minor: The documentation article component now supports disabling the semantic rendering using a falsy value in https://github.com/hydephp/develop/pull/1566
- Minor: Changed the default build task message to make it more concise in https://github.com/hydephp/develop/pull/1659
- Minor: Data collection files are now validated for syntax errors during discovery in https://github.com/hydephp/develop/pull/1732
- Minor: Methods in the `Includes` facade now return `HtmlString` objects instead of `string` in https://github.com/hydephp/develop/pull/1738. For more information, see below.
- The `hasFeature` method on the Hyde facade and HydeKernel now only accepts a Feature enum value instead of a string for its parameter.
- Changed how the documentation search is generated, to be an `InMemoryPage` instead of a post-build task.
- Media asset files are now copied using the new build task instead of the deprecated `BuildService::transferMediaAssets()` method.
Expand Down Expand Up @@ -223,6 +224,22 @@ For example, if you triggered the media transfer with a build service method cal
(new TransferMediaAssets())->run();
```

### Includes facade changes

The following methods in the `Includes` facade now return `HtmlString` objects instead of `string`:

- `Includes::html()`
- `Includes::blade()`
- `Includes::markdown()`

- This means that you no longer need to use `{!! !!}` to render the output of these methods in Blade templates, instead just use `{{ }}`.
- If you have used the return value of these methods in custom code, you may need to adjust your code to work with the new return type.
- **Note:** Remember that this means that includes are **no longer escaped** by default, so make sure to escape the output if necessary, for example if the content is user-generated.
- (Use `{{ e(Includes::html('foo')) }}` instead of `{{ Includes::html('foo') }}` to escape the output, matching the previous behavior.)

For more information, see the RFC that proposed this change: https://github.com/hydephp/develop/issues/1734
The RFC wasd implemented in https://github.com/hydephp/develop/pull/1738

### DataCollection API changes

The DataCollection feature has been reworked to improve the developer experience and make it more consistent with the rest of the API.
Expand Down
8 changes: 8 additions & 0 deletions docs/digging-deeper/helpers.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ All methods will return `null` if the file does not exist.
However, you can supply a default value as the second argument to be used instead.
Remember that Markdown and Blade defaults will also be rendered to HTML.

Includes are particularly useful in Blade views, as you can echo them directly. You do not need to import the namespace, as it is already aliased.

```blade
<footer>
{{ Includes::markdown('footer.md') }}
</footer>
```

```php
use Hyde\Support\Includes;

Expand Down
25 changes: 13 additions & 12 deletions packages/framework/src/Support/Includes.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Hyde\Hyde;
use Hyde\Facades\Filesystem;
use Illuminate\Support\HtmlString;
use Hyde\Markdown\Models\MarkdownDocument;
use Hyde\Markdown\Models\Markdown;
use Illuminate\Support\Facades\Blade;
Expand Down Expand Up @@ -61,53 +62,53 @@ public static function get(string $filename, ?string $default = null): ?string
*
* @param string $filename The name of the partial file, with or without the extension.
* @param string|null $default The default value to return if the partial is not found.
* @return string|null The raw contents of the partial file, or the default value if not found.
* @return HtmlString|null The contents of the partial file, or the default value if not found.
*/
public static function html(string $filename, ?string $default = null): ?string
public static function html(string $filename, ?string $default = null): ?HtmlString
{
$path = static::normalizePath($filename, '.html');

if (! Filesystem::exists($path)) {
return $default === null ? null : $default;
return $default === null ? null : new HtmlString($default);
}

return Filesystem::get($path);
return new HtmlString(Filesystem::get($path));
}

/**
* Get the rendered Markdown of a partial file in the includes directory.
*
* @param string $filename The name of the partial file, with or without the extension.
* @param string|null $default The default value to return if the partial is not found.
* @return string|null The rendered contents of the partial file, or the default value if not found.
* @return HtmlString|null The rendered contents of the partial file, or the default value if not found.
*/
public static function markdown(string $filename, ?string $default = null): ?string
public static function markdown(string $filename, ?string $default = null): ?HtmlString
{
$path = static::normalizePath($filename, '.md');

if (! Filesystem::exists($path)) {
return $default === null ? null : trim(Markdown::render($default, MarkdownDocument::class));
return $default === null ? null : new HtmlString(trim(Markdown::render($default, MarkdownDocument::class)));
}

return trim(Markdown::render(Filesystem::get($path), MarkdownDocument::class));
return new HtmlString(trim(Markdown::render(Filesystem::get($path), MarkdownDocument::class)));
}

/**
* Get the rendered Blade of a partial file in the includes directory.
*
* @param string $filename The name of the partial file, with or without the extension.
* @param string|null $default The default value to return if the partial is not found.
* @return string|null The rendered contents of the partial file, or the default value if not found.
* @return HtmlString|null The rendered contents of the partial file, or the default value if not found.
*/
public static function blade(string $filename, ?string $default = null): ?string
public static function blade(string $filename, ?string $default = null): ?HtmlString
{
$path = static::normalizePath($filename, '.blade.php');

if (! Filesystem::exists($path)) {
return $default === null ? null : Blade::render($default);
return $default === null ? null : new HtmlString(Blade::render($default));
}

return Blade::render(Filesystem::get($path));
return new HtmlString(Blade::render(Filesystem::get($path)));
}

protected static function normalizePath(string $filename, string $extension): string
Expand Down
39 changes: 23 additions & 16 deletions packages/framework/tests/Feature/IncludesFacadeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Hyde\Support\Includes;
use Hyde\Hyde;
use Hyde\Testing\TestCase;
use Illuminate\Support\HtmlString;
use Illuminate\Support\Facades\Blade;

/**
Expand Down Expand Up @@ -58,63 +59,63 @@ public function testHtmlReturnsRenderedPartial()
{
$expected = '<h1>foo bar</h1>';
file_put_contents(Hyde::path('resources/includes/foo.html'), '<h1>foo bar</h1>');
$this->assertSame($expected, Includes::html('foo.html'));
$this->assertHtmlStringIsSame($expected, Includes::html('foo.html'));
Filesystem::unlink('resources/includes/foo.html');
}

public function testHtmlReturnsEfaultValueWhenNotFound()
{
$this->assertNull(Includes::html('foo.html'));
$this->assertSame('<h1>default</h1>', Includes::html('foo.html', '<h1>default</h1>'));
$this->assertHtmlStringIsSame('<h1>default</h1>', Includes::html('foo.html', '<h1>default</h1>'));
}

public function testHtmlWithAndWithoutExtension()
{
file_put_contents(Hyde::path('resources/includes/foo.html'), '# foo bar');
$this->assertSame(Includes::html('foo.html'), Includes::html('foo'));
$this->assertHtmlStringIsSame(Includes::html('foo.html'), Includes::html('foo'));
Filesystem::unlink('resources/includes/foo.html');
}

public function testMarkdownReturnsRenderedPartial()
{
$expected = '<h1>foo bar</h1>';
file_put_contents(Hyde::path('resources/includes/foo.md'), '# foo bar');
$this->assertSame($expected, Includes::markdown('foo.md'));
$this->assertHtmlStringIsSame($expected, Includes::markdown('foo.md'));
Filesystem::unlink('resources/includes/foo.md');
}

public function testMarkdownReturnsRenderedDefaultValueWhenNotFound()
{
$this->assertNull(Includes::markdown('foo.md'));
$this->assertSame('<h1>default</h1>', Includes::markdown('foo.md', '# default'));
$this->assertHtmlStringIsSame('<h1>default</h1>', Includes::markdown('foo.md', '# default'));
}

public function testMarkdownWithAndWithoutExtension()
{
file_put_contents(Hyde::path('resources/includes/foo.md'), '# foo bar');
$this->assertSame(Includes::markdown('foo.md'), Includes::markdown('foo'));
$this->assertHtmlStringIsSame(Includes::markdown('foo.md'), Includes::markdown('foo'));
Filesystem::unlink('resources/includes/foo.md');
}

public function testBladeReturnsRenderedPartial()
{
$expected = 'foo bar';
file_put_contents(Hyde::path('resources/includes/foo.blade.php'), '{{ "foo bar" }}');
$this->assertSame($expected, Includes::blade('foo.blade.php'));
$this->assertHtmlStringIsSame($expected, Includes::blade('foo.blade.php'));
Filesystem::unlink('resources/includes/foo.blade.php');
}

public function testBladeWithAndWithoutExtension()
{
file_put_contents(Hyde::path('resources/includes/foo.blade.php'), '# foo bar');
$this->assertSame(Includes::blade('foo.blade.php'), Includes::blade('foo'));
$this->assertHtmlStringIsSame(Includes::blade('foo.blade.php'), Includes::blade('foo'));
Filesystem::unlink('resources/includes/foo.blade.php');
}

public function testBladeReturnsRenderedDefaultValueWhenNotFound()
{
$this->assertNull(Includes::blade('foo.blade.php'));
$this->assertSame('default', Includes::blade('foo.blade.php', '{{ "default" }}'));
$this->assertHtmlStringIsSame('default', Includes::blade('foo.blade.php', '{{ "default" }}'));
}

public function testTorchlightAttributionIsNotInjectedToMarkdownPartials()
Expand All @@ -126,8 +127,8 @@ public function testTorchlightAttributionIsNotInjectedToMarkdownPartials()

$attribution = 'Syntax highlighting by <a href="https://torchlight.dev/" rel="noopener nofollow">Torchlight.dev</a>';

$this->assertStringNotContainsString($attribution, $rendered);
$this->assertSame("<p>$placeholder</p>", $rendered);
$this->assertStringNotContainsString($attribution, $rendered->toHtml());
$this->assertHtmlStringIsSame("<p>$placeholder</p>", $rendered);
}

public function testAdvancedMarkdownDocumentIsCompiledToHtml()
Expand Down Expand Up @@ -192,7 +193,7 @@ public function testAdvancedMarkdownDocumentIsCompiledToHtml()
HTML;

$this->file('resources/includes/advanced.md', $markdown);
$this->assertSame($expected, Includes::markdown('advanced.md'));
$this->assertHtmlStringIsSame($expected, Includes::markdown('advanced.md'));
}

public function testAdvancedBladePartialIsCompiledToHtml()
Expand All @@ -217,7 +218,7 @@ public function testAdvancedBladePartialIsCompiledToHtml()
HTML;

$this->file('resources/includes/advanced.blade.php', $blade);
$this->assertSame($expected, Includes::blade('advanced.blade.php'));
$this->assertHtmlStringIsSame($expected, Includes::blade('advanced.blade.php'));
}

public function testIncludesUsageFromBladeView()
Expand Down Expand Up @@ -257,11 +258,17 @@ public function testIncludesUsageFromBladeView()
<h1>Compiled Markdown</h1>
// With escaped
&lt;h1&gt;Literal HTML&lt;/h1&gt;
&lt;h1&gt;Rendered Blade&lt;/h1&gt;
&lt;h1&gt;Compiled Markdown&lt;/h1&gt;
<h1>Literal HTML</h1>
<h1>Rendered Blade</h1>
<h1>Compiled Markdown</h1>
HTML;

$this->assertSame($expected, Blade::render($view));
}

protected function assertHtmlStringIsSame(string|HtmlString $expected, mixed $actual): void
{
$this->assertInstanceOf(HtmlString::class, $actual);
$this->assertSame((string) $expected, $actual->toHtml());
}
}
29 changes: 18 additions & 11 deletions packages/framework/tests/Unit/IncludesFacadeUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Hyde\Hyde;
use Hyde\Support\Includes;
use Hyde\Testing\UnitTestCase;
use Illuminate\Support\HtmlString;
use Illuminate\Support\Facades\Blade;
use Illuminate\Filesystem\Filesystem;

Expand Down Expand Up @@ -82,7 +83,7 @@ public function testHtmlReturnsRenderedPartial()
$filesystem->shouldReceive('get')->with($this->includesPath($filename))->andReturn($expected);
});

$this->assertSame($expected, Includes::html($filename));
$this->assertHtmlStringIsSame($expected, Includes::html($filename));
}

public function testHtmlReturnsDefaultValueWhenNotFound()
Expand All @@ -95,7 +96,7 @@ public function testHtmlReturnsDefaultValueWhenNotFound()
});

$this->assertNull(Includes::html($filename));
$this->assertSame($default, Includes::html($filename, $default));
$this->assertHtmlStringIsSame($default, Includes::html($filename, $default));
}

public function testHtmlWithAndWithoutExtension()
Expand All @@ -108,7 +109,7 @@ public function testHtmlWithAndWithoutExtension()
$filesystem->shouldReceive('get')->with($this->includesPath($filename))->andReturn($content);
});

$this->assertSame(Includes::html('foo.html'), Includes::html('foo'));
$this->assertHtmlStringIsSame(Includes::html('foo.html'), Includes::html('foo'));
}

public function testMarkdownReturnsRenderedPartial()
Expand All @@ -123,7 +124,7 @@ public function testMarkdownReturnsRenderedPartial()
$filesystem->shouldReceive('get')->with($this->includesPath($filename))->andReturn($content);
});

$this->assertSame($expected, Includes::markdown($filename));
$this->assertHtmlStringIsSame($expected, Includes::markdown($filename));
}

public function testMarkdownReturnsRenderedDefaultValueWhenNotFound()
Expand All @@ -137,7 +138,7 @@ public function testMarkdownReturnsRenderedDefaultValueWhenNotFound()
});

$this->assertNull(Includes::markdown($filename));
$this->assertSame($expected, Includes::markdown($filename, $default));
$this->assertHtmlStringIsSame($expected, Includes::markdown($filename, $default));
}

public function testMarkdownWithAndWithoutExtension()
Expand All @@ -152,9 +153,9 @@ public function testMarkdownWithAndWithoutExtension()
$filesystem->shouldReceive('get')->with($this->includesPath($filename))->andReturn($content);
});

$this->assertSame($expected, Includes::markdown('foo.md'));
$this->assertSame(Includes::markdown('foo.md'), Includes::markdown('foo'));
$this->assertSame(Includes::markdown('foo.md'), Includes::markdown('foo.md'));
$this->assertHtmlStringIsSame($expected, Includes::markdown('foo.md'));
$this->assertHtmlStringIsSame(Includes::markdown('foo.md'), Includes::markdown('foo'));
$this->assertHtmlStringIsSame(Includes::markdown('foo.md'), Includes::markdown('foo.md'));
}

public function testBladeReturnsRenderedPartial()
Expand All @@ -171,7 +172,7 @@ public function testBladeReturnsRenderedPartial()
Blade::shouldReceive('render')->with($content)->andReturn($expected);
});

$this->assertSame($expected, Includes::blade($filename));
$this->assertHtmlStringIsSame($expected, Includes::blade($filename));
}

public function testBladeWithAndWithoutExtension()
Expand All @@ -187,7 +188,7 @@ public function testBladeWithAndWithoutExtension()
Blade::shouldReceive('render')->with($content)->andReturn($expected);
});

$this->assertSame(Includes::blade('foo.blade.php'), Includes::blade('foo'));
$this->assertHtmlStringIsSame(Includes::blade('foo.blade.php'), Includes::blade('foo'));
}

public function testBladeReturnsDefaultValueWhenNotFound()
Expand All @@ -203,7 +204,7 @@ public function testBladeReturnsDefaultValueWhenNotFound()
});

$this->assertNull(Includes::blade($filename));
$this->assertSame($expected, Includes::blade($filename, $default));
$this->assertHtmlStringIsSame($expected, Includes::blade($filename, $default));
}

protected function mockFilesystem(Closure $config): void
Expand All @@ -219,4 +220,10 @@ protected function includesPath(string $filename): string
{
return Hyde::path('resources/includes/'.$filename);
}

protected function assertHtmlStringIsSame(string|HtmlString $expected, mixed $actual): void
{
$this->assertInstanceOf(HtmlString::class, $actual);
$this->assertSame((string) $expected, $actual->toHtml());
}
}

0 comments on commit 4122038

Please sign in to comment.