-
-
Notifications
You must be signed in to change notification settings - Fork 436
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
Feature: ESM Import Maps #3990
Draft
colinmollenhour
wants to merge
9
commits into
OpenMage:main
Choose a base branch
from
colinmollenhour:esm-import-map
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Feature: ESM Import Maps #3990
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
Component: Core
Relates to Mage_Core
Component: Page
Relates to Mage_Page
labels
May 13, 2024
fballiano
reviewed
May 14, 2024
fballiano
reviewed
May 14, 2024
fballiano
reviewed
May 14, 2024
fballiano
reviewed
May 14, 2024
Co-authored-by: Fabrizio Balliano <[email protected]>
Oops, forgot to check for styling issues.. old habits, sorry.. Thanks for the suggestions, @fballiano |
colinmollenhour
commented
May 14, 2024
colinmollenhour
commented
May 14, 2024
colinmollenhour
commented
May 14, 2024
colinmollenhour
commented
May 14, 2024
Closing due to limited interest and unproven utility. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a native method to make use of Import Maps in OpenMage. Import maps are very well supported by now (not IE11) and allow one to use modern Javascript (ES Modules) without any compilation or build tools like Webpack.
Note on browser support: All major browsers support the "import" statement and "importmap" type, but what is not widely supported are the very rarely used keywords "with" and "assert" as well as the "src=" attribute, but basic inline importmaps are well supported.
Discussion
I've opened a discussion topic to discuss the merits of the feature, alternatives, etc. Please use the PR primarily for code suggestions.
For example, see 65380d6 which adds a Vue app to the footer and renders some dynamic content from a JS file imported from the skin directory.
Previously this would require special "AMD", "UMD" or "Global" builds of projects that not all modules provide and some may pollute the global namespace and often require you to preload the assets even if they aren't used. Import maps allow you to define the import sources and then the browser will fetch them as-needed and all of the other reasons that imports are superior that have been developing for the last 10+ years.
import echarts from 'https://cdn.jsdelivr.net/npm/[email protected]/+esm'
but this leaves urls with version specifiers scattered all over your code - with this there is one place to specify the import urlBy supporting import maps as a proper asset type in OpenMage, different modules can add the same imports without duplicating them (assuming the versions don't conflict). If there were conflicts that one really needed to support they could craft their own
importmap.json
file to support "scopes" as that is supported by the import map spec and respected by this PR.How to generate dependencies
This update does not aim to be highly opinionated about how you maintain your import maps, just provide a way to include them in the head tag without conflicts.
In a complex project you might have many dependencies that have their own dependencies and you'd rather a tool like
npm
handle them than try to do it manually. Handling dependencies is left up to the end user but one way would be as follows:js/my-bundle/package.json
npm install
, creatingpackage-lock.json
npx importly < package-lock.json > importmap.json
<action method="addStaticImportMap"><name>my</name><file>my-bundle/importmap.json</file></action>
(An alternative to importly is JSPM:Generator.)
By using
importmap.json
files with the "scopes" keyword it would be possible for extensions to not conflict with one another at all. However, this PR is not opinionated in this regard. Establishing some conventions could help avoid future conflicts but I'm just trying to get the capability in the core first.Overrides
Although the spec doesn't support multiple import maps, this OpenMage feature does by simply reading the JSON of each import map and merging it with the others and the single imports. In this way, one can override imports from other modules by clobbering its namespace.
API
Public layout methods added to
page/html_head
block:TODO