From d2014eb09c65a5c65c9b48c1a240cf3c42ca2f20 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Tue, 3 Oct 2023 11:31:38 -0400 Subject: [PATCH] [AssetMapper] Put importmap in polyfill so it can be hosted locally easily --- .../Bundle/FrameworkBundle/CHANGELOG.md | 1 + .../DependencyInjection/Configuration.php | 4 +-- .../FrameworkExtension.php | 3 +- .../DependencyInjection/ConfigurationTest.php | 4 +-- .../Component/AssetMapper/CHANGELOG.md | 1 + .../ImportMap/ImportMapManager.php | 1 - .../ImportMap/ImportMapRenderer.php | 24 +++++++++++-- .../Tests/ImportMap/ImportMapRendererTest.php | 34 +++++++++++++++++-- 8 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 3ba4fa7165926..2e22a3816414b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -31,6 +31,7 @@ CHANGELOG * Deprecate the `framework.asset_mapper.provider` config option * Add `--exclude` option to the `cache:pool:clear` command * Add parameters deprecations to the output of `debug:container` command + * Change `framework.asset_mapper.importmap_polyfill` from a URL to the name of an item in the importmap 6.3 --- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 4410181f9127b..7d386b543c706 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -924,8 +924,8 @@ private function addAssetMapperSection(ArrayNodeDefinition $rootNode, callable $ ->defaultValue('%kernel.project_dir%/importmap.php') ->end() ->scalarNode('importmap_polyfill') - ->info('URL of the ES Module Polyfill to use, false to disable. Defaults to using a CDN URL.') - ->defaultValue(null) + ->info('The importmap name that will be used to load the polyfill. Set to false to disable.') + ->defaultValue('es-module-shims') ->end() ->arrayNode('importmap_script_attributes') ->info('Key-value pair of attributes to add to script tags output for the importmap.') diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 7592ed38d2e48..8cc03e94f0d2f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -33,7 +33,6 @@ use Symfony\Component\Asset\PackageInterface; use Symfony\Component\AssetMapper\AssetMapper; use Symfony\Component\AssetMapper\Compiler\AssetCompilerInterface; -use Symfony\Component\AssetMapper\ImportMap\ImportMapManager; use Symfony\Component\BrowserKit\AbstractBrowser; use Symfony\Component\Cache\Adapter\AdapterInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; @@ -1378,7 +1377,7 @@ private function registerAssetMapperConfiguration(array $config, ContainerBuilde $container ->getDefinition('asset_mapper.importmap.renderer') - ->replaceArgument(3, $config['importmap_polyfill'] ?? ImportMapManager::POLYFILL_URL) + ->replaceArgument(3, $config['importmap_polyfill']) ->replaceArgument(4, $config['importmap_script_attributes']) ; } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 1e251bbd4480f..6ed9c24780df7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -132,7 +132,7 @@ public function testAssetMapperCanBeEnabled() 'missing_import_mode' => 'warn', 'extensions' => [], 'importmap_path' => '%kernel.project_dir%/importmap.php', - 'importmap_polyfill' => null, + 'importmap_polyfill' => 'es-module-shims', 'vendor_dir' => '%kernel.project_dir%/assets/vendor', 'importmap_script_attributes' => [], ]; @@ -668,7 +668,7 @@ protected static function getBundleDefaultConfig() 'missing_import_mode' => 'warn', 'extensions' => [], 'importmap_path' => '%kernel.project_dir%/importmap.php', - 'importmap_polyfill' => null, + 'importmap_polyfill' => 'es-module-shims', 'vendor_dir' => '%kernel.project_dir%/assets/vendor', 'importmap_script_attributes' => [], ], diff --git a/src/Symfony/Component/AssetMapper/CHANGELOG.md b/src/Symfony/Component/AssetMapper/CHANGELOG.md index 012dde82a8a26..628b3c1484360 100644 --- a/src/Symfony/Component/AssetMapper/CHANGELOG.md +++ b/src/Symfony/Component/AssetMapper/CHANGELOG.md @@ -17,6 +17,7 @@ CHANGELOG * Allow specifying packages to update for the `importmap:update` command * Add a `importmap:audit` command to check for security vulnerability advisories in dependencies * Add a `importmap:outdated` command to check for outdated packages + * Change the polyfill used for the importmap renderer from a URL to an entry in the importmap 6.3 --- diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php index 6144eab323f37..33a9a3bbb0403 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php @@ -24,7 +24,6 @@ */ class ImportMapManager { - public const POLYFILL_URL = 'https://ga.jspm.io/npm:es-module-shims@1.7.2/dist/es-module-shims.js'; public const IMPORT_MAP_CACHE_FILENAME = 'importmap.json'; public const ENTRYPOINT_CACHE_FILENAME_PATTERN = 'entrypoint.%s.json'; diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php index 8e54da6c0ba60..e3ce84da76c13 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php @@ -27,11 +27,13 @@ */ class ImportMapRenderer { + private const DEFAULT_ES_MODULE_SHIMS_POLYFILL_URL = 'https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js'; + public function __construct( private readonly ImportMapManager $importMapManager, private readonly ?Packages $assetPackages = null, private readonly string $charset = 'UTF-8', - private readonly string|false $polyfillUrl = ImportMapManager::POLYFILL_URL, + private readonly string|false $polyfillImportName = false, private readonly array $scriptAttributes = [], private readonly ?RequestStack $requestStack = null, ) { @@ -45,6 +47,7 @@ public function render(string|array $entryPoint, array $attributes = []): string $importMap = []; $modulePreloads = []; $cssLinks = []; + $polyFillPath = null; foreach ($importMapData as $importName => $data) { $path = $data['path']; @@ -53,6 +56,12 @@ public function render(string|array $entryPoint, array $attributes = []): string $path = $this->assetPackages->getUrl(ltrim($path, '/')); } + // if this represents the polyfill, hide it from the import map + if ($importName === $this->polyfillImportName) { + $polyFillPath = $path; + continue; + } + $preload = $data['preload'] ?? false; if ('css' !== $data['type']) { $importMap[$importName] = $path; @@ -87,8 +96,17 @@ public function render(string|array $entryPoint, array $attributes = []): string HTML; - if ($this->polyfillUrl) { - $url = $this->escapeAttributeValue($this->polyfillUrl); + if (false !== $this->polyfillImportName && null === $polyFillPath) { + if ('es-module-shims' !== $this->polyfillImportName) { + throw new \InvalidArgumentException(sprintf('The JavaScript module polyfill was not found in your import map. Either disable the polyfill or run "php bin/console importmap:require "%s"" to install it.', $this->polyfillImportName)); + } + + // a fallback for the default polyfill in case it's not in the importmap + $polyFillPath = self::DEFAULT_ES_MODULE_SHIMS_POLYFILL_URL; + } + + if ($polyFillPath) { + $url = $this->escapeAttributeValue($polyFillPath); $output .= << 'https://cdn.example.com/assets/remote-d1g35t.js', 'type' => 'js', ], + 'es-module-shim' => [ + 'path' => 'https://ga.jspm.io/npm:es-module-shims', + 'type' => 'js', + ], ]); $assetPackages = $this->createMock(Packages::class); @@ -64,11 +68,14 @@ public function testBasicRender() return '/subdirectory/'.$path; }); - $renderer = new ImportMapRenderer($importMapManager, $assetPackages); + $renderer = new ImportMapRenderer($importMapManager, $assetPackages, polyfillImportName: 'es-module-shim'); $html = $renderer->render(['app']); $this->assertStringContainsString('', $html); + // and is hidden from the import map + $this->assertStringNotContainsString('"es-module-shim"', $html); $this->assertStringContainsString('import \'app\';', $html); // preloaded js file @@ -93,9 +100,26 @@ public function testNoPolyfill() $this->assertStringNotContainsString('https://ga.jspm.io/npm:es-module-shims', $renderer->render([])); } + public function testDefaultPolyfillUsedIfNotInImportmap() + { + $importMapManager = $this->createMock(ImportMapManager::class); + $importMapManager->expects($this->once()) + ->method('getImportMapData') + ->with(['app']) + ->willReturn([]); + + $renderer = new ImportMapRenderer( + $importMapManager, + $this->createMock(Packages::class), + polyfillImportName: 'es-module-shims', + ); + $html = $renderer->render(['app']); + $this->assertStringContainsString('