Skip to content

Commit

Permalink
bug #52508 [AssetMapper] Fix jsdelivr import parsing with no imported…
Browse files Browse the repository at this point in the history
… value (weaverryan)

This PR was merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Fix jsdelivr import parsing with no imported value

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #52467
| License       | MIT

Another new import syntax found by `@tacman`! I think this one is kind of a bug in the library - a module environment should not `import 'foo'` (i.e. import a module and not actually use any value), but our job is just to find these, not judge them ;).

FYI - I have an issue on jsdelivr proposing that they create an API endpoint to expose this info, so we don't need to parse it. They agree and already were thinking about this - jsdelivr/jsdelivr#18538 - so this may be something helpful in the future.

Cheers!

Commits
-------

dc1b27d [AssetMapper] Adding import regex support for not importing any value
  • Loading branch information
nicolas-grekas committed Nov 10, 2023
2 parents 278b203 + dc1b27d commit 3128c60
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
Expand Up @@ -26,7 +26,7 @@ final class JsDelivrEsmResolver implements PackageResolverInterface
public const URL_PATTERN_DIST = self::URL_PATTERN_DIST_CSS.'/+esm';
public const URL_PATTERN_ENTRYPOINT = 'https://data.jsdelivr.com/v1/packages/npm/%s@%s/entrypoints';

public const IMPORT_REGEX = '{from"/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm"}';
public const IMPORT_REGEX = '#(?:import\s*(?:(?:\{[^}]*\}|\w+|\*\s*as\s+\w+)\s*\bfrom\s*)?|export\s*(?:\{[^}]*\}|\*)\s*from\s*)("/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm")#';

private HttpClientInterface $httpClient;

Expand Down Expand Up @@ -221,9 +221,9 @@ private function fetchPackageRequirementsFromImports(string $content): array
// imports from jsdelivr follow a predictable format
preg_match_all(self::IMPORT_REGEX, $content, $matches);
$dependencies = [];
foreach ($matches[1] as $index => $packageName) {
$version = $matches[2][$index] ?: null;
$packageName .= $matches[3][$index]; // add the path if any
foreach ($matches[2] as $index => $packageName) {
$version = $matches[3][$index] ?: null;
$packageName .= $matches[4][$index]; // add the path if any

$dependencies[] = new PackageRequireOptions($packageName, $version);
}
Expand All @@ -239,10 +239,11 @@ private function fetchPackageRequirementsFromImports(string $content): array
private function makeImportsBare(string $content, array &$dependencies): string
{
$content = preg_replace_callback(self::IMPORT_REGEX, function ($matches) use (&$dependencies) {
$packageName = $matches[1].$matches[3]; // add the path if any
$packageName = $matches[2].$matches[4]; // add the path if any
$dependencies[] = $packageName;

return sprintf('from"%s"', $packageName);
// replace the "/npm/package@version/+esm" with "package@version"
return str_replace($matches[1], sprintf('"%s"', $packageName), $matches[0]);
}, $content);

// source maps are not also downloaded - so remove the sourceMappingURL
Expand Down
Expand Up @@ -463,12 +463,12 @@ public function testImportRegex(string $subject, array $expectedPackages)
$expectedVersions[] = $packageData[1];
}
$actualNames = [];
foreach ($matches[1] as $i => $name) {
$actualNames[] = $name.$matches[3][$i];
foreach ($matches[2] as $i => $name) {
$actualNames[] = $name.$matches[4][$i];
}

$this->assertSame($expectedNames, $actualNames);
$this->assertSame($expectedVersions, $matches[2]);
$this->assertSame($expectedVersions, $matches[3]);
}

public static function provideImportRegex(): iterable
Expand Down Expand Up @@ -529,6 +529,26 @@ public static function provideImportRegex(): iterable
['prosemirror-state/php/strings/sprintf', '1.4.3'],
],
];

yield 'import without importing a value' => [
'import "/npm/jquery@3.7.1/+esm";',
[
['jquery', '3.7.1'],
],
];

yield 'multiple imports and exports with and without values' => [
'import"/npm/jquery@3.7.1/+esm";import e from"/npm/datatables.net-bs5@1.13.7/+esm";export{default}from"/npm/datatables.net-bs5@1.13.7/+esm";import"/npm/datatables.net-select@1.7.0/+esm";
/*! Bootstrap 5 styling wrapper for Select
* © SpryMedia Ltd - datatables.net/license
*/',
[
['jquery', '3.7.1'],
['datatables.net-bs5', '1.13.7'],
['datatables.net-bs5', '1.13.7'],
['datatables.net-select', '1.7.0'],
],
];
}

private static function createRemoteEntry(string $importName, string $version, ImportMapType $type = ImportMapType::JS, string $packageSpecifier = null): ImportMapEntry
Expand Down

0 comments on commit 3128c60

Please sign in to comment.