8000 [AssetMapper] Fix in-file imports to resolve via filesystem by weaverryan · Pull Request #52349 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[AssetMapper] Fix in-file imports to resolve via filesystem #52349

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 28, 2023
Merged

[AssetMapper] Fix in-file imports to resolve via filesystem #52349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
[AssetMapper] Fix in-file imports to resolve via filesystem
  • Loading branch information
@weaverryan @fabpot
weaverryan authored and fabpot committed Oct 28, 2023
commit 659c635a7cbdcc1a3e8fa5c25c1742cb19d70775
88 changes: 0 additions & 88 deletions src/Symfony/Component/AssetMapper/Compiler/AssetCompilerPathResolverTrait.php
View file
Open in desktop

This file was deleted.

17 changes: 11 additions & 6 deletions src/Symfony/Component/AssetMapper/Compiler/CssAssetUrlCompiler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\AssetMapper\AssetMapperInterface;
use Symfony\Component\AssetMapper\Exception\RuntimeException;
use Symfony\Component\AssetMapper\MappedAsset;
use Symfony\Component\Filesystem\Path;

/**
* Resolves url() paths in CSS files.
Expand All @@ -23,8 +24,6 @@
*/
final class CssAssetUrlCompiler implements AssetCompilerInterface
{
use AssetCompilerPathResolverTrait;

// https://regex101.com/r/BOJ3vG/1
public const ASSET_URL_PATTERN = '/url\(\s*["\']?(?!(?:\/|\#|%23|data|http|\/\/))([^"\'\s?#)]+)([#?][^"\')]+)?\s*["\']?\)/';

Expand All @@ -38,23 +37,29 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
{
return preg_replace_callback(self::ASSET_URL_PATTERN, function ($matches) use ($asset, $assetMapper) {
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like we can ignore this. In practice, our preg_replace_callback() will return a string. But if there is some change we want to make to satisfy psalm, we can.

$resolvedPath = $this->resolvePath(\dirname($asset->logicalPath), $matches[1]);
$resolvedSourcePath = Path::join(\dirname($asset->sourcePath), $matches[1]);
Copy link
Member Author

Choose a reason for hiding this comment

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

A bit unrelated: let's let Path do the work

} catch (RuntimeException $e) {
$this->handleMissingImport(sprintf('Error processing import in "%s": ', $asset->sourcePath).$e->getMessage(), $e);

return $matches[0];
}
$dependentAsset = $assetMapper->getAsset($resolvedPath);
$dependentAsset = $assetMapper->getAssetFromSourcePath($resolvedSourcePath);

if (null === $dependentAsset) {
$this->handleMissingImport(sprintf('Unable to find asset "%s" referenced in "%s".', $matches[1], $asset->sourcePath));
$message = sprintf('Unable to find asset "%s" referenced in "%s". The file "%s" ', $matches[1], $asset->sourcePath, $resolvedSourcePath);
if (is_file($resolvedSourcePath)) {
$message .= 'exists, but it is not in a mapped asset path. Add it to the "paths" config.';
} else {
$message .= 'does not exist.';
}
$this->handleMissingImport($message);

// return original, unchanged path
return $matches[0];
}

$asset->addDependency($dependentAsset);
$relativePath = $this->createRelativePath($asset->publicPathWithoutDigest, $dependentAsset->publicPath);
$relativePath = Path::makeRelative($dependentAsset->publicPath, \dirname($asset->publicPathWithoutDigest));

return 'url("'.$relativePath.'")';
}, $content);
Expand Down
33 changes: 19 additions & 14 deletions src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\AssetMapper\ImportMap\ImportMapConfigReader;
use Symfony\Component\AssetMapper\ImportMap\JavaScriptImport;
use Symfony\Component\AssetMapper\MappedAsset;
use Symfony\Component\Filesystem\Path;

/**
* Resolves import paths in JS files.
Expand All @@ -26,8 +27,6 @@
*/
final class JavaScriptImportPathCompiler implements AssetCompilerInterface
{
use AssetCompilerPathResolverTrait;

// https://regex101.com/r/fquriB/1
private const IMPORT_PATTERN = '/(?:import\s*(?:(?:\*\s*as\s+\w+|[\w\s{},*]+)\s*from\s*)?|\bimport\()\s*[\'"`](\.\/[^\'"`]+|(\.\.\/)*[^\'"`]+)[\'"`]\s*[;\)]?/m';

Expand Down Expand Up @@ -80,7 +79,7 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
}

// support possibility where the final public files have moved relative to each other
$relativeImportPath = $this->createRelativePath($asset->publicPathWithoutDigest, $dependentAsset->publicPathWithoutDigest);
$relativeImportPath = Path::makeRelative($dependentAsset->publicPathWithoutDigest, \dirname($asset->publicPathWithoutDigest));
$relativeImportPath = $this->makeRelativeForJavaScript($relativeImportPath);

return str_replace($importedModule, $relativeImportPath, $fullImportString);
Expand Down Expand Up @@ -155,7 +154,7 @@ private function findAssetForBareImport(string $importedModule, AssetMapperInter
private function findAssetForRelativeImport(string $importedModule, MappedAsset $asset, AssetMapperInterface $assetMapper): ?MappedAsset
{
try {
$resolvedPath = $this->resolvePath(\dirname($asset->logicalPath), $importedModule);
$resolvedSourcePath = Path::join(\dirname($asset->sourcePath), $importedModule);
} catch (RuntimeException $e) {
// avoid warning about vendor imports - these are often comments
if (!$asset->isVendor) {
Expand All @@ -165,26 +164,32 @@ private function findAssetForRelativeImport(string $importedModule, MappedAsset
return null;
}

$dependentAsset = $assetMapper->getAsset($resolvedPath);
$dependentAsset = $assetMapper->getAssetFromSourcePath($resolvedSourcePath);

if ($dependentAsset) {
return $dependentAsset;
}

// avoid warning about vendor imports - these are often comments
if ($asset->isVendor) {
return null;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This just represents a reorg so we can return earlier


$message = sprintf('Unable to find asset "%s" imported from "%s".', $importedModule, $asset->sourcePath);

try {
if (null !== $assetMapper->getAsset(sprintf('%s.js', $resolvedPath))) {
$message .= sprintf(' Try adding ".js" to the end of the import - i.e. "%s.js".', $importedModule);
if (is_file($resolvedSourcePath)) {
$message .= sprintf('The file "%s" exists, but it is not in a mapped asset path. Add it to the "paths" config.', $resolvedSourcePath);
} else {
try {
if (null !== $assetMapper->getAssetFromSourcePath(sprintf('%s.js', $resolvedSourcePath))) {
$message .= sprintf(' Try adding ".js" to the end of the import - i.e. "%s.js".', $importedModule);
}
} catch (CircularAssetsException) {
// avoid circular error if there is self-referencing import comments
}
} catch (CircularAssetsException) {
// avoid circular error if there is self-referencing import comments
}

// avoid warning about vendor imports - these are often comments
if (!$asset->isVendor) {
$this->handleMissingImport($message);
}
$this->handleMissingImport($message);

return null;
}
Expand Down
9 changes: 4 additions & 5 deletions src/Symfony/Component/AssetMapper/Compiler/SourceMappingUrlsCompiler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\AssetMapper\AssetMapperInterface;
use Symfony\Component\AssetMapper\MappedAsset;
use Symfony\Component\Filesystem\Path;

/**
* Rewrites already-existing source map URLs to their final digested path.
Expand All @@ -21,8 +22,6 @@
*/
final class SourceMappingUrlsCompiler implements AssetCompilerInterface
{
use AssetCompilerPathResolverTrait;

private const SOURCE_MAPPING_PATTERN = '/^(\/\/|\/\*)# sourceMappingURL=(.+\.map)/m';

public function supports(MappedAsset $asset): bool
Expand All @@ -33,16 +32,16 @@ public function supports(MappedAsset $asset): bool
public function compile(string $content, MappedAsset $asset, AssetMapperInterface $assetMapper): string
{
return preg_replace_callback(self::SOURCE_MAPPING_PATTERN, function ($matches) use ($asset, $assetMapper) {
$resolvedPath = $this->resolvePath(\dirname($asset->logicalPath), $matches[2]);
$resolvedPath = Path::join(\dirname($asset->sourcePath), $matches[2]);

$dependentAsset = $assetMapper->getAsset($resolvedPath);
$dependentAsset = $assetMapper->getAssetFromSourcePath($resolvedPath);
if (!$dependentAsset) {
// return original, unchanged path
return $matches[0];
}

$asset->addDependency($dependentAsset);
$relativePath = $this->createRelativePath($asset->publicPathWithoutDigest, $dependentAsset->publicPath);
$relativePath = Path::makeRelative($dependentAsset->publicPath, \dirname($asset->publicPathWithoutDigest));

return $matches[1].'# sourceMappingURL='.$relativePath;
}, $content);
Expand Down
133 changes: 0 additions & 133 deletions src/Symfony/Component/AssetMapper/Tests/Compiler/AssetCompilerPathResolverTraitTest.php
View file
Open in desktop

This file was deleted.

Loading
0