Skip to content
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.

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

[FrameworkBundle] remove support for preloading ESM using headers #50476

Closed
wants to merge 1 commit into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented May 30, 2023

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets n/a
License MIT
Doc PR n/a

Sorry to be that late on this one.
It looks like preloading ESM using HTTP headers (and so using 103 Early Hints) doesn't work, at least for now: WICG/import-maps#273 (comment)

ImportMap must be defined before downloading any ESM, and by definition HTTP headers are sent before importmap definition (it's currently not possible to define importmap using headers).

I don't know why I didn't caught this during my previous testing session, but now I get this error in the console when using this feature (which, according to the previously mentioned issue, is expected):

An import map is added after module script load was triggered.
Uncaught TypeError: Failed to resolve module specifier "app". Relative references must start with either "/", "./", or "../".

This patch simply removes this feature, which is entirely non-functional and misleading for the end user.

@carsonbot carsonbot added this to the 6.3 milestone May 30, 2023
@dunglas dunglas changed the title [FrameworkBundle] remove support for preloading ESM modules using hea… [FrameworkBundle] remove support for preloading ESM modules using headers May 30, 2023
@dunglas dunglas changed the title [FrameworkBundle] remove support for preloading ESM modules using headers [FrameworkBundle] remove support for preloading ESM using headers May 30, 2023
@nicolas-grekas
Copy link
Member

Thank you @dunglas.

nicolas-grekas added a commit that referenced this pull request May 30, 2023
…headers (dunglas)

This PR was merged into the 6.3 branch.

Discussion
----------

[FrameworkBundle] remove support for preloading ESM using headers

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | n/a
| License       | MIT
| Doc PR        | n/a

Sorry to be that late on this one.
It looks like preloading ESM using HTTP headers (and so using 103 Early Hints) doesn't work, at least for now: WICG/import-maps#273 (comment)

ImportMap must be defined before downloading any ESM, and by definition HTTP headers are sent before importmap definition (it's currently not possible to define importmap using headers).

I don't know why I didn't caught this during my previous testing session, but now I get this error in the console when using this feature (which, according to the previously mentioned issue, is expected):

```
An import map is added after module script load was triggered.
Uncaught TypeError: Failed to resolve module specifier "app". Relative references must start with either "/", "./", or "../".
```

This patch simply removes this feature, which is entirely non-functional and misleading for the end user.

Commits
-------

7776c28 [FrameworkBundle] remove support for preloading ESM modules using headers
@dunglas dunglas deleted the fix/remove-103-importmap branch May 30, 2023 15:41
@fabpot fabpot mentioned this pull request May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants