OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
866 stars 436 forks source link

Feature: ESM Import Maps #3990

Open colinmollenhour opened 4 months ago

colinmollenhour commented 4 months ago

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 https://github.com/OpenMage/magento-lts/commit/65380d6dda839e1070381fff374ae8a4e6d5bfa2 which adds a Vue app to the footer and renders some dynamic content from a JS file imported from the skin directory. image image

image

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.

By 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:

  1. Create a npm module at js/my-bundle/package.json
  2. Add dependencies and npm install, creating package-lock.json
  3. Generate an importmap file using importly: npx importly < package-lock.json > importmap.json
  4. Add the importmap to your layout XML: <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:

addStaticImportMap($name, $path, $devPath = null, $referenceName = '*', $before = false)
addSkinImportMap($name, $path, $devPath = null, $referenceName = '*', $before = false)
addStaticImport($specifier, $fileOrUrl, $devFileOrUrl = null, $referenceName = '*', $before = false)
addSkinImport($specifier, $fileOrUrl, $devFileOrUrl = null, $referenceName = '*', $before = false)

TODO

colinmollenhour commented 4 months ago

Oops, forgot to check for styling issues.. old habits, sorry.. Thanks for the suggestions, @fballiano

colinmollenhour commented 2 months ago

Closing due to limited interest and unproven utility.