Skip to content

Commit

Permalink
bug #50231 [AssetMapper] Fixing 2 bugs related to the compile command…
Browse files Browse the repository at this point in the history
… and importmaps (weaverryan)

This PR was squashed before being merged into the 6.3 branch.

Discussion
----------

[AssetMapper] Fixing 2 bugs related to the compile command and importmaps

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | none
| Tickets       | none
| License       | MIT
| Doc PR        | not needed

Fixes 2 bugs:

A) Running `assetmap:compile` gives you a `public/assets/importmap.json` for fast importmap dumping on production. But, running `assetmap:compile` again later would re-use the `importmap.json` info to create the new `importmap.json`... meaning it would never update after the first compile.

B) The importmap process generates 2 pieces of information: (A) the importmap and (B) which files from the importmap should be preloaded. When we use `assetmap:compile`, we need to dump both pieces of info. So, we now also dump an `importmap.preload.json` file. These are TWO files (and not just one) because we avoid parsing `importmap.json` at runtime: we read the file and dump it straight out. We DO need to parse the "preload" file. So, I've kept them separate.

Blocked by #50219, which has some changes that will fix the tests here.

Cheers!

Commits
-------

f9f7274 [AssetMapper] Fixing 2 bugs related to the compile command and importmaps
  • Loading branch information
nicolas-grekas committed May 5, 2023
2 parents 584c210 + f9f7274 commit ba94e95
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/AssetMapper/AssetMapper.php
Expand Up @@ -267,7 +267,7 @@ private function loadManifest(): array
if (null === $this->manifestData) {
$path = $this->getPublicAssetsFilesystemPath().'/'.self::MANIFEST_FILE_NAME;

if (!file_exists($path)) {
if (!is_file($path)) {
$this->manifestData = [];
} else {
$this->manifestData = json_decode(file_get_contents($path), true);
Expand Down
Expand Up @@ -54,7 +54,7 @@ public function find(string $logicalPath): ?string
}

$file = rtrim($path, '/').'/'.$localLogicalPath;
if (file_exists($file)) {
if (is_file($file)) {
return realpath($file);
}
}
Expand Down
Expand Up @@ -66,13 +66,54 @@ protected function execute(InputInterface $input, OutputInterface $output): int
throw new InvalidArgumentException(sprintf('The public directory "%s" does not exist.', $publicDir));
}

$outputDir = $publicDir.$this->assetMapper->getPublicPrefix();
if ($input->getOption('clean')) {
$outputDir = $publicDir.$this->assetMapper->getPublicPrefix();
$io->comment(sprintf('Cleaning <info>%s</info>', $outputDir));
$this->filesystem->remove($outputDir);
$this->filesystem->mkdir($outputDir);
}

$manifestPath = $publicDir.$this->assetMapper->getPublicPrefix().AssetMapper::MANIFEST_FILE_NAME;
if (is_file($manifestPath)) {
$this->filesystem->remove($manifestPath);
}
$manifest = $this->createManifestAndWriteFiles($io, $publicDir);
$this->filesystem->dumpFile($manifestPath, json_encode($manifest, \JSON_PRETTY_PRINT));
$io->comment(sprintf('Manifest written to <info>%s</info>', $manifestPath));

$importMapPath = $outputDir.ImportMapManager::IMPORT_MAP_FILE_NAME;
if (is_file($importMapPath)) {
$this->filesystem->remove($importMapPath);
}
$this->filesystem->dumpFile($importMapPath, $this->importMapManager->getImportMapJson());

$importMapPreloadPath = $outputDir.ImportMapManager::IMPORT_MAP_PRELOAD_FILE_NAME;
if (is_file($importMapPreloadPath)) {
$this->filesystem->remove($importMapPreloadPath);
}
$this->filesystem->dumpFile(
$importMapPreloadPath,
json_encode($this->importMapManager->getModulesToPreload(), \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES)
);
$io->comment(sprintf('Import map written to <info>%s</info> and <info>%s</info> for quick importmap dumping onto the page.', $this->shortenPath($importMapPath), $this->shortenPath($importMapPreloadPath)));

if ($this->isDebug) {
$io->warning(sprintf(
'You are compiling assets in development. Symfony will not serve any changed assets until you delete the "%s" directory.',
$this->shortenPath($outputDir)
));
}

return 0;
}

private function shortenPath(string $path): string
{
return str_replace($this->projectDir.'/', '', $path);
}

private function createManifestAndWriteFiles(SymfonyStyle $io, string $publicDir): array
{
$allAssets = $this->assetMapper->allAssets();

$io->comment(sprintf('Compiling <info>%d</info> assets to <info>%s%s</info>', \count($allAssets), $publicDir, $this->assetMapper->getPublicPrefix()));
Expand All @@ -88,23 +129,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->filesystem->dumpFile($targetPath, $asset->getContent());
$manifest[$asset->logicalPath] = $asset->getPublicPath();
}
ksort($manifest);

$manifestPath = $publicDir.$this->assetMapper->getPublicPrefix().AssetMapper::MANIFEST_FILE_NAME;
$this->filesystem->dumpFile($manifestPath, json_encode($manifest, \JSON_PRETTY_PRINT));
$io->comment(sprintf('Manifest written to <info>%s</info>', $manifestPath));

$importMapPath = $publicDir.$this->assetMapper->getPublicPrefix().ImportMapManager::IMPORT_MAP_FILE_NAME;
$this->filesystem->dumpFile($importMapPath, $this->importMapManager->getImportMapJson());
$io->comment(sprintf('Import map written to <info>%s</info>', $importMapPath));

if ($this->isDebug) {
$io->warning(sprintf(
'You are compiling assets in development. Symfony will not serve any changed assets until you delete %s and %s.',
$manifestPath,
$importMapPath
));
}

return 0;
return $manifest;
}
}
Expand Up @@ -49,6 +49,7 @@ class ImportMapManager
*/
private const PACKAGE_PATTERN = '/^(?:https?:\/\/[\w\.-]+\/)?(?:(?<registry>\w+):)?(?<package>(?:@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*)(?:@(?<version>[\w\._-]+))?(?:(?<subpath>\/.*))?$/';
public const IMPORT_MAP_FILE_NAME = 'importmap.json';
public const IMPORT_MAP_PRELOAD_FILE_NAME = 'importmap.preload.json';

private array $importMapEntries;
private array $modulesToPreload;
Expand Down Expand Up @@ -125,9 +126,11 @@ private function buildImportMapJson(): void
return;
}

$dumpedPath = $this->assetMapper->getPublicAssetsFilesystemPath().'/'.self::IMPORT_MAP_FILE_NAME;
if (file_exists($dumpedPath)) {
$this->json = file_get_contents($dumpedPath);
$dumpedImportMapPath = $this->assetMapper->getPublicAssetsFilesystemPath().'/'.self::IMPORT_MAP_FILE_NAME;
$dumpedModulePreloadPath = $this->assetMapper->getPublicAssetsFilesystemPath().'/'.self::IMPORT_MAP_PRELOAD_FILE_NAME;
if (is_file($dumpedImportMapPath) && is_file($dumpedModulePreloadPath)) {
$this->json = file_get_contents($dumpedImportMapPath);
$this->modulesToPreload = json_decode(file_get_contents($dumpedModulePreloadPath), true, 512, \JSON_THROW_ON_ERROR);

return;
}
Expand Down
Expand Up @@ -40,14 +40,20 @@ public function testAssetsAreCompiled()
{
$application = new Application($this->kernel);

$targetBuildDir = $this->kernel->getProjectDir().'/public/assets';
// put old "built" versions to make sure the system skips using these
$this->filesystem->mkdir($targetBuildDir);
file_put_contents($targetBuildDir.'/manifest.json', '{}');
file_put_contents($targetBuildDir.'/importmap.json', '{"imports": {}}');
file_put_contents($targetBuildDir.'/importmap.preload.json', '{}');

$command = $application->find('asset-map:compile');
$tester = new CommandTester($command);
$res = $tester->execute([]);
$this->assertSame(0, $res);
// match Compiling \d+ assets
$this->assertMatchesRegularExpression('/Compiling \d+ assets/', $tester->getDisplay());

$targetBuildDir = $this->kernel->getProjectDir().'/public/assets';
$this->assertFileExists($targetBuildDir.'/subdir/file5-f4fdc37375c7f5f2629c5659a0579967.js');
$this->assertSame(<<<EOF
import '../file4.js';
Expand All @@ -57,23 +63,35 @@ public function testAssetsAreCompiled()

$finder = new Finder();
$finder->in($targetBuildDir)->files();
$this->assertCount(9, $finder);
$this->assertCount(10, $finder);
$this->assertFileExists($targetBuildDir.'/manifest.json');

$expected = [
$this->assertSame([
'already-abcdefVWXYZ0123456789.digested.css',
'file1.css',
'file2.js',
'file3.css',
'subdir/file6.js',
'subdir/file5.js',
'file4.js',
'already-abcdefVWXYZ0123456789.digested.css',
];
$actual = array_keys(json_decode(file_get_contents($targetBuildDir.'/manifest.json'), true));
sort($expected);
sort($actual);
'subdir/file5.js',
'subdir/file6.js',
], array_keys(json_decode(file_get_contents($targetBuildDir.'/manifest.json'), true)));

$this->assertSame($expected, $actual);
$this->assertFileExists($targetBuildDir.'/importmap.json');
$actualImportMap = json_decode(file_get_contents($targetBuildDir.'/importmap.json'), true);
$this->assertSame([
'@hotwired/stimulus',
'lodash',
'file6',
'/assets/subdir/file5.js', // imported by file6
'/assets/file4.js', // imported by file5
], array_keys($actualImportMap['imports']));

$this->assertFileExists($targetBuildDir.'/importmap.preload.json');
$actualPreload = json_decode(file_get_contents($targetBuildDir.'/importmap.preload.json'), true);
$this->assertCount(4, $actualPreload);
$this->assertStringStartsWith('https://unpkg.com/@hotwired/stimulus', $actualPreload[0]);
$this->assertStringStartsWith('/assets/subdir/file6-', $actualPreload[1]);
$this->assertStringStartsWith('/assets/subdir/file5-', $actualPreload[2]);
$this->assertStringStartsWith('/assets/file4-', $actualPreload[3]);
}
}
Expand Up @@ -84,6 +84,9 @@ public function testGetImportMapJsonUsesDumpedFile()
'@hotwired/stimulus' => 'https://unpkg.com/@hotwired/stimulus@3.2.1/dist/stimulus.js',
'app' => '/assets/app-ea9ebe6156adc038aba53164e2be0867.js',
]], json_decode($manager->getImportMapJson(), true));
$this->assertEquals([
'/assets/app-ea9ebe6156adc038aba53164e2be0867.js',
], $manager->getModulesToPreload());
}

/**
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/AssetMapper/Tests/fixtures/importmap.php
@@ -0,0 +1,25 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

return [
'@hotwired/stimulus' => [
'url' => 'https://unpkg.com/@hotwired/stimulus@3.2.1/dist/stimulus.js',
'preload' => true,
],
'lodash' => [
'url' => 'https://ga.jspm.io/npm:lodash@4.17.21/lodash.js',
'preload' => false,
],
'file6' => [
'path' => 'subdir/file6.js',
'preload' => true,
],
];
@@ -0,0 +1,3 @@
[
"/assets/app-ea9ebe6156adc038aba53164e2be0867.js"
]

0 comments on commit ba94e95

Please sign in to comment.