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

Incorrect Sass binary path construction results in repetitive download of the binary #66

Open
FlyingDR opened this issue Apr 7, 2024 · 1 comment

Comments

@FlyingDR
Copy link

FlyingDR commented Apr 7, 2024

The current implementation of the construction of the path for Sass binary relies on the expectation that the main entry point is always named sass:

private function getDefaultBinaryPath(): string
{
return $this->binaryDownloadDir.'/dart-sass/sass';
}

But this is not true for Windows, where it is a sass.bat. Effectively it results in a re-download of the Sass distributive for each build due to this logic:

$binary = $this->getDefaultBinaryPath();
if (!is_file($binary)) {
$this->downloadExecutable();
}

Correct implementation of the getDefaultBinaryPath method would be:

private function getDefaultBinaryPath(): string
{
    return $this->binaryDownloadDir.'/dart-sass/sass'.(str_contains(strtolower(\PHP_OS), 'win') ? '.bat' : '');
}

There is also one more related and Windows-specific issue:

Normally after download bundle tests that download results in the appearance of the Sass binary and throws an exception if it is not so:

$binaryPath = $this->getDefaultBinaryPath();
if (!is_file($binaryPath)) {
throw new \Exception(sprintf('Could not find downloaded binary in "%s".', $binaryPath));
}

However, for Windows, no such check is applied because there is a separate branch for archive extraction that ends with unconditional return.

if ($isZip) {
if (!\extension_loaded('zip')) {
throw new \Exception('Cannot unzip the downloaded sass binary. Please install the "zip" PHP extension.');
}
$archive = new \ZipArchive();
$archive->open($targetPath);
$archive->extractTo($this->binaryDownloadDir);
$archive->close();
unlink($targetPath);
return;

Because of this, in case if Sass binary download will not cause the Sass compiler to actually appear in the filesystem it will not be reported.

@bocharsky-bw
Copy link
Member

Hey @FlyingDR ,

Thank you for reporting these Windows-specific bugs! Would you like to create PRs for them? It would be much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants