8000 [AssetMapper] Import-Polyfill throws error in JS-Console. · Issue #52783 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[AssetMapper] Import-Polyfill throws error in JS-Console. #52783

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

Jump to bottom
Closed
Chris53897 opened this issue Nov 29, 2023 · 5 comments
Closed

[AssetMapper] Import-Polyfill throws error in JS-Console. #52783

Chris53897 opened this issue Nov 29, 2023 · 5 comments
Labels
AssetMapper Bug Status: Needs Review

Comments

@Chris53897
Copy link
Contributor

Symfony version(s) affected

6.4.0

Description

Relates to #52547

I could not find a way to make the polyfill for importmap work.

I am aware that the behaviour changed between 6.3 and 6.4.
https://symfony.com/doc/6.4/frontend/asset_mapper.html#framework-asset-mapper-importmap-polyfill

In 6.3 i used the files from here.
// https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js
// https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js.map

These files do not have the problematic last line from the lastest shim.
export{t as default};
Is this because of the /+esm

a) No shim: Works

framework:
    asset_mapper:
        importmap_polyfill: false

b) Per Default (not setting importmap_polyfill) the shim is the 1.8.2 (or actual version) from CDN.
https://cdn.jsdelivr.net/npm/es-module-shims@1.8.2/+esm

framework:
    asset_mapper:
        ...

But with this i get this error in the browser js console.
es-module-shims.index-e76dd15dae6b25cfe767df2e7fe0ec9e.js:7 Uncaught SyntaxError: Unexpected token 'export' (at es-module-shims.index-e76dd15dae6b25cfe767df2e7fe0ec9e.js:7:33789)

c) Default suggested installation (same result as b) )
php bin/console importmap:require es-module-shims

framework:
    asset_mapper:
        importmap_polyfill: 'es-module-shims'

d) WORKS: Use my local files from Symfony 6.3 and copy them into /assets/lib/es-module-shims/
// https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js
// https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js.map

Run:
php bin/console importmap:require "es-module-shims" --path=./assets/lib/es-module-shims/es-module-shims.js

framework:
    asset_mapper:
        importmap_polyfill: 'es-module-shims'

importmap.php

'es-module-shims' => [
        'path' => './assets/lib/es-module-shims/es-module-shims.js',
    ],```


### How to reproduce

See above

### Possible Solution

_No response_

### Additional Context

_No response_
@Chris53897 Chris53897 added the Bug label Nov 29, 2023
@smnandre
Copy link
Member

Could you give the versions of your browser, and of asset-mapper installed ? That could help narrow the problem

@Chris53897
Copy link
Contributor Author

Chrome 119
Firefox 120

@smnandre
Copy link
Member

c) there is a problem there indeed

b), i think you still had a 'es-module-shims' in your importmap.php, because the default URL of the remote polyfill is 1.8.0 in Symfony 6.4

@weaverryan
Copy link
Member

Not sure I how missed this.

A) The default config should work. It will load from the same CDN url as 6.3 - https://ga.jspm.io/npm:es-module-shims@1.8.0/dist/es-module-shims.js

B) However, when you importmap:require es-module-shims, the downloaded file has an extra export {t as default};. I knew that was there and was unnecessary, but I thought it was harmless. But I DO see the error. Indeed - it's caused by the /+esm at the end of the URL. This is the ONE special case where we don't want that.

We already have special handling in ImportMapRenderer for es-module-shims. To fix this, I'd recommend one more piece of special code in JsDelivrEsmResolver.

If $packageName === 'es-module-shims then we should use the URL_PATTERN_DIST_CSS (which does not have the /+esm:

symfony/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php

Line 81 in 83d0106

$pattern = str_ends_with($filePath, '.css') ? self::URL_PATTERN_DIST_CSS : self::URL_PATTERN_DIST;

(we also have this logic on line 172, so we could isolate it into a private method).

Can someone make a PR?

@hashbanged hashbanged mentioned this issue Dec 12, 2023
Merged
@hashbanged
Copy link
Contributor

PR #53003 is a shot at it.

fabpot added a commit that referenced this issue Dec 20, 2023
@fabpot
bug #53003 [AssetMapper] Fix URL pattern when importing es-module-shi…
5534989 
…ms (hashbanged)

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

Discussion
----------

[AssetMapper] Fix URL pattern when importing es-module-shims

This fixes a JS console syntax error when importing the `es-module-shims` polyfills package

| Q             | A
| ------------- | ---
| Branch?       | 6.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix #52783  <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

Requiring `es-module-shims` using AssetMapper causes a trailing `export {t as default};` in the downloaded file. The recommendation in #52783 is to be explicit about checking if the package is the shims/polyfill, and, if so, to use the CSS URL pattern when building the HTTP request for the package. This PR adds a new method for handling this case, and it centralizes the logic for determining if the URL pattern should be `URL_PATTERN_DIST`|`URL_PATTERN_DIST_CSS`
<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Commits
-------

1472137 [AssetMapper] Fix URL pattern when importing es-module-shims
@fabpot fabpot closed this as completed Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
AssetMapper Bug Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@fabpot @weaverryan @smnandre @xabbuh @Chris53897 @carsonbot @hashbanged
0