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

Add support for named converted output files #30

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/AssetMapper/SassCssCompiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ public function supports(MappedAsset $asset): bool

public function compile(string $content, MappedAsset $asset, AssetMapperInterface $assetMapper): string
{
$cssFile = $this->sassBuilder->guessCssNameFromSassFile($asset->sourcePath, $this->cssPathDirectory);
$fileName = $this->sassBuilder->getIdentifierByLogicalPath($asset->logicalPath);

$cssFile = SassBuilder::guessCssNameFromSassFile($asset->sourcePath, $this->cssPathDirectory, $fileName);

$asset->addFileDependency($cssFile);

Expand Down
2 changes: 1 addition & 1 deletion src/DependencyInjection/SymfonycastsSassExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->end()
->validate()
->ifTrue(static function (array $paths): bool {
if (1 === \count($paths)) {
if (1 === \count($paths) || !array_is_list($paths)) {
Copy link

Choose a reason for hiding this comment

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

If you want to allow using maps and not just lists, you need to use ->useAttributeAsKey. Otherwise, merging multiple config files (note that for this matter, a when@prod section counts as a separate config for the merging process) together won't preserve keys (and devs using a XML config file would not be able to specify a key)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how I would define that?

$rootNode
    ->children()
        ->arrayNode('root_sass')
            ->info('Path to your Sass root file')
            ->cannotBeEmpty()
            ->useAttributeAsKey('???')  // <--- here?
            ->scalarPrototype()
                ->end()
            ->validate()
                ...

With or without ->useAttributeAsKey(), and manually testing it with an extra when@dev config for example, does not change the array passed to the validate-callback function? And what would be the $name variable to pass to ->useAttributeAsKey()?

Can you point me to an example definition?

Copy link

Choose a reason for hiding this comment

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

@evertharmeling you would have to define a when@dev section (or a separate file) that also configures this root_sass node, so that some merging actually happens (if the value is defined only in 1 place, the merging is bypassed entirely, which will preserve keys even if the node is not configured for that).

For the name being passed, this would be the name of the attribute used in an XML config (where it cannot be an array key). In this case, output looks like a good option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing with:

config/packages/symfony_casts.yaml:

symfonycasts_sass:
    root_sass:
        website: 'assets/website/scss/app.scss'

config/packages/test.yaml:

symfonycasts_sass:
    root_sass:
        test: 'assets/website/scss/app.scss'
        website: 'assets/test/scss/app.scss'

when@dev:
    symfonycasts_sass:
        root_sass:
            test2: 'assets/website/scss/app.scss'

Running symfony console debug:config SymfonycastsSassBundle:

Current configuration for "SymfonycastsSassBundle"
==================================================

symfonycasts_sass:
    root_sass:
        website: assets/test/scss/app.scss
        test: assets/website/scss/app.scss
        test2: assets/website/scss/app.scss
    binary: null
    embed_sourcemap: true

Even replacing an entry with same key website seems to work?

I'm fine with adding ->useAttributeAsKey('output'), but can't reproduce it with these examples or am I still overlooking something?

return false;
}

Expand Down
29 changes: 25 additions & 4 deletions src/SassBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ public function runBuild(bool $watch): Process
public function getScssCssTargets(): array
{
$targets = [];
foreach ($this->sassPaths as $sassPath) {
foreach ($this->sassPaths as $fileName => $sassPath) {
if (!is_file($sassPath)) {
throw new \Exception(sprintf('Could not find Sass file: "%s"', $sassPath));
}

$targets[] = $sassPath.':'.$this->guessCssNameFromSassFile($sassPath, $this->cssPath);
$targets[] = $sassPath.':'.self::guessCssNameFromSassFile($sassPath, $this->cssPath, $fileName);
}

return $targets;
Expand All @@ -90,13 +90,34 @@ public function setOutput(SymfonyStyle $output): void
/**
* @internal
*/
public static function guessCssNameFromSassFile(string $sassFile, string $outputDirectory): string
public static function guessCssNameFromSassFile(string $sassFile, string $outputDirectory, string|int $fileName = null): string
{
$fileName = basename($sassFile, '.scss');
if (null === $fileName || \is_int($fileName)) {
$fileName = basename($sassFile, '.scss');
}

return $outputDirectory.'/'.$fileName.'.output.css';
}

public function getIdentifierByLogicalPath(string $path): ?string
{
if (array_is_list($this->sassPaths)) {
return null;
}

foreach ($this->sassPaths as $identifier => $configuredSassPath) {
// as the configured paths include the project dir, we need to subtract it to be able to compare the paths
$pathPrefix = $this->projectRootDir.'/assets/';
$logicalPath = substr($configuredSassPath, \strlen($pathPrefix));

if ($path === $logicalPath) {
return $identifier;
}
}

return null;
}

private function createBinary(): SassBinary
{
return new SassBinary($this->projectRootDir.'/var', $this->binaryPath, $this->output);
Expand Down
120 changes: 120 additions & 0 deletions tests/AssetMapper/SassCssCompilerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
<?php

declare(strict_types=1);

/*
* This file is part of the SymfonyCasts SassBundle package.
* Copyright (c) SymfonyCasts <https://symfonycasts.com/>
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfonycasts\SassBundle\Tests\AssetMapper;

use PHPUnit\Framework\TestCase;
use Symfony\Component\AssetMapper\AssetMapperInterface;
use Symfony\Component\AssetMapper\MappedAsset;
use Symfonycasts\SassBundle\AssetMapper\SassCssCompiler;
use Symfonycasts\SassBundle\SassBuilder;

final class SassCssCompilerTest extends TestCase
{
private const CSS_DIR = __DIR__.'/../fixtures/var/sass';

protected function setUp(): void
{
if (!is_dir(self::CSS_DIR)) {
mkdir(self::CSS_DIR, 0777, true);
}
}

public function testCompileSingleSassPath(): void
{
$scssFile = __DIR__.'/../fixtures/assets/app.scss';
$scssPaths = [
$scssFile,
];
$cssFile = self::CSS_DIR.'/app.output.css';

$compiler = new SassCssCompiler(
$scssPaths,
self::CSS_DIR,
$this->createSassBuilder($scssPaths)
);

$mappedAsset = new MappedAsset(
'app.scss',
$scssFile,
'app.css'
);

file_put_contents($cssFile, <<<EOF
p {
color: red;
}
EOF
);

$compiledContent = $compiler->compile(
file_get_contents($scssFile),
$mappedAsset,
$this->createMock(AssetMapperInterface::class)
);

$this->assertStringEqualsFile(
$cssFile,
$compiledContent
);
}

public function testCompileNamedSassPath()
{
$scssFile = __DIR__.'/../fixtures/assets/admin/app.scss';

$scssPaths = [
'admin' => $scssFile,
];
$cssFile = self::CSS_DIR.'/admin.output.css';

$compiler = new SassCssCompiler(
$scssPaths,
self::CSS_DIR,
$this->createSassBuilder($scssPaths)
);

$mappedAsset = new MappedAsset(
'admin/app.scss',
$scssFile,
'admin.css'
);

file_put_contents($cssFile, <<<EOF
p {
color: blue;
}
EOF
);

$compiledContent = $compiler->compile(
file_get_contents($scssFile),
$mappedAsset,
$this->createMock(AssetMapperInterface::class)
);

$this->assertStringEqualsFile(
$cssFile,
$compiledContent
);
}

private function createSassBuilder(array $sassPaths): SassBuilder
{
return new SassBuilder(
$sassPaths,
self::CSS_DIR,
__DIR__.'/../fixtures',
null,
false
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* file that was distributed with this source code.
*/

namespace Symfonycasts\SassBundle\Tests;
namespace Symfonycasts\SassBundle\Tests\DependencyInjection;

use Matthias\SymfonyConfigTest\PhpUnit\ConfigurationTestCaseTrait;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -47,6 +47,18 @@ public function testMultipleSassRootPaths(): void
]);
}

public function testMultipleSassRootPathsWithIdentifier(): void
{
$this->assertConfigurationIsValid([
'symfonycasts_sass' => [
'root_sass' => [
'website' => '%kernel.project_dir%/assets/scss/app.scss',
'admin' => '%kernel.project_dir%/assets/admin/scss/app.scss',
],
],
]);
}

public function testMultipleSassRootPathsWithSameFilename(): void
{
$this->assertConfigurationIsInvalid([
Expand Down
33 changes: 31 additions & 2 deletions tests/SassBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ class SassBuilderTest extends TestCase
{
protected function tearDown(): void
{
unlink(__DIR__.'/fixtures/assets/app.output.css');
unlink(__DIR__.'/fixtures/assets/app.output.css.map');
@unlink(__DIR__.'/fixtures/assets/app.output.css');
@unlink(__DIR__.'/fixtures/assets/app.output.css.map');

@unlink(__DIR__.'/fixtures/assets/foo.output.css');
@unlink(__DIR__.'/fixtures/assets/foo.output.css.map');

@unlink(__DIR__.'/fixtures/assets/bar.output.css');
@unlink(__DIR__.'/fixtures/assets/bar.output.css.map');
}

public function testIntegration(): void
Expand All @@ -37,4 +43,27 @@ public function testIntegration(): void
$this->assertFileExists(__DIR__.'/fixtures/assets/app.output.css');
$this->assertStringContainsString('color: red;', file_get_contents(__DIR__.'/fixtures/assets/app.output.css'));
}

public function testPathsConfigWithKeys(): void
{
$builder = new SassBuilder(
[
'foo' => __DIR__.'/fixtures/assets/app.scss',
'bar' => __DIR__.'/fixtures/assets/admin/app.scss',
],
__DIR__.'/fixtures/assets',
__DIR__.'/fixtures',
null,
false
);

$process = $builder->runBuild(false);
$process->wait();

$this->assertTrue($process->isSuccessful());
$this->assertFileExists(__DIR__.'/fixtures/assets/foo.output.css');
$this->assertFileExists(__DIR__.'/fixtures/assets/bar.output.css');
$this->assertStringContainsString('color: red;', file_get_contents(__DIR__.'/fixtures/assets/foo.output.css'));
$this->assertStringContainsString('color: blue;', file_get_contents(__DIR__.'/fixtures/assets/bar.output.css'));
}
}
5 changes: 5 additions & 0 deletions tests/fixtures/assets/admin/app.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$color: blue;

p {
color: $color;
}