-
-
Notifications
You must be signed in to change notification settings - Fork 26
Add binary version configuration #42
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
5621b5d
771b3e3
d6a74c5
8403495
0e958fc
cda8510
5b522c0
f92e9d8
f9f8acc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,6 +142,17 @@ To see the full config from this bundle, run: | |
|
|
||
| The main option is ``root_sass`` option, which defaults to ``assets/styles/app.scss``. This represents the source Sass file. | ||
|
|
||
| Using a different version | ||
| -------------------------- | ||
| This bundle installs for you a default version. However, if you want an explicit version of Dart Sass you can instruct the bundle to download that version, set the ``version`` option: | ||
|
|
||
| .. code-block:: yaml | ||
|
|
||
| symfonycasts_sass: | ||
| version: 1.69.0 | ||
|
|
||
| When you change this version, it will not be upgraded automatically. Remove the `var/dart-sass` directory first to rebuild with the configured version. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about using the binary version in the directory name containing the binary, so when we change the version it know that it should download the new version?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be sweet! It would solve WTF moments when it's cached with older version 👍
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds like a great solution, I'll implement and document it! |
||
|
|
||
| Using a different binary | ||
| -------------------------- | ||
|
|
||
|
|
@@ -151,3 +162,5 @@ This bundle already installed for you the right binary. However, if you already | |
|
|
||
| symfonycasts_sass: | ||
| binary: 'node_modules/.bin/sass' | ||
|
|
||
| This configuration overrides any specific version configuration. | ||
|
bocharsky-bw marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,8 @@ public function load(array $configs, ContainerBuilder $container): void | |
| ->replaceArgument(0, $config['root_sass']) | ||
| ->replaceArgument(1, '%kernel.project_dir%/var/sass') | ||
| ->replaceArgument(3, $config['binary']) | ||
| ->replaceArgument(4, $config['embed_sourcemap']) | ||
| ->replaceArgument(4, $config['version']) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should keep the name binary_version. This is the naming we use in the code and it make think more clear.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree |
||
| ->replaceArgument(5, $config['embed_sourcemap']) | ||
| ; | ||
|
|
||
| $container->findDefinition('sass.css_asset_compiler') | ||
|
|
@@ -81,6 +82,10 @@ public function getConfigTreeBuilder(): TreeBuilder | |
| ->info('The Sass binary to use') | ||
| ->defaultNull() | ||
| ->end() | ||
| ->scalarNode('version') | ||
| ->info('The Sass binary version to download') | ||
| ->defaultNull() | ||
|
bocharsky-bw marked this conversation as resolved.
|
||
| ->end() | ||
|
dorxy marked this conversation as resolved.
Outdated
|
||
| ->scalarNode('embed_sourcemap') | ||
| ->info('Whether to embed the sourcemap in the compiled CSS. By default, enabled only when debug mode is on.') | ||
| ->defaultValue('%kernel.debug%') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ public function __construct( | |
| private readonly string $cssPath, | ||
| private readonly string $projectRootDir, | ||
| private readonly ?string $binaryPath, | ||
| private readonly ?string $binaryVersion, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, this will lead to BC breaks. What about to move this to the end instead? |
||
| private readonly bool $embedSourcemap, | ||
| ) { | ||
| } | ||
|
|
@@ -99,6 +100,6 @@ public static function guessCssNameFromSassFile(string $sassFile, string $output | |
|
|
||
| private function createBinary(): SassBinary | ||
| { | ||
| return new SassBinary($this->projectRootDir.'/var', $this->binaryPath, $this->output); | ||
| return new SassBinary($this->projectRootDir.'/var', $this->binaryPath, $this->binaryVersion, $this->output); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| <?php | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this test should be on the |
||
|
|
||
| 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; | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
| use Symfony\Component\Filesystem\Filesystem; | ||
| use Symfony\Component\Process\Process; | ||
| use Symfonycasts\SassBundle\SassBinary; | ||
|
|
||
| class VersionTest extends TestCase | ||
| { | ||
| protected function tearDown(): void | ||
| { | ||
| if (file_exists(__DIR__.'/fixtures/var')) { | ||
| $filesystem = new Filesystem(); | ||
| $filesystem->remove(__DIR__.'/fixtures/var'); | ||
| } | ||
| } | ||
|
|
||
| public function testVersionDownloaded(): void | ||
| { | ||
| $testedVersion = '1.69.5'; // This should differ from the SassBinary::DEFAULT_VERSION constant | ||
| $binary = new SassBinary(__DIR__.'/fixtures/var/version', null, $testedVersion); | ||
|
|
||
| $binary->downloadExecutable(); | ||
| $sassVersionProcess = new Process([__DIR__.'/fixtures/var/version/dart-sass/sass', '--version']); | ||
| $sassVersionProcess->run(); | ||
|
|
||
| $this->assertSame(trim($sassVersionProcess->getOutput(), \PHP_EOL), $testedVersion); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not the
path to binary versionbut simply thebinary version?