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

import map generator should make a folder alias instead of direct file path mappings #296

Open
trusktr opened this issue Oct 10, 2023 · 10 comments

Comments

@trusktr
Copy link

trusktr commented Oct 10, 2023

Currently the import map generator generators things like this:

            "@lume/autolayout/es/AutoLayout.js": "https://ga.jspm.io/npm:@lume/[email protected]/es/AutoLayout.js",
            "@lume/custom-attributes/dist/index.js": "https://ga.jspm.io/npm:@lume/[email protected]/dist/index.js",
            "@lume/three-projected-material/dist/ProjectedMaterial.js": "https://ga.jspm.io/npm:@lume/[email protected]/dist/ProjectedMaterial.js",

playground

This makes the import map inflexible: if someone modifies source code to import different files, it can fail.

For example, if source code starts to import an additional @lume/autolayout/es/Foo.js, due to updating the version of one lib in the import map, it will fail.

Instead, the import map generator can make folder aliases, so that the result is like this:

            "@lume/autolayout/": "https://ga.jspm.io/npm:@lume/[email protected]/",
            "@lume/custom-attributes/": "https://ga.jspm.io/npm:@lume/[email protected]/",
            "@lume/three-projected-material/": "https://ga.jspm.io/npm:@lume/[email protected]/",

With this type of generated import map, the import map is more resilient to such failures, rather then the app breaking.

In some cases the person changing source code is not the same person getting a generated import map, so the current behavior can lead to a failure when the end user of a library updates a version number in the import map.

The user can and should generate a new import map. But it would be nice to avoid it when it is not necessary

@guybedford
Copy link
Member

The import map generator actually does do this automatically, but only when there is more than one entry for a given folder path.

@trusktr
Copy link
Author

trusktr commented Oct 25, 2023

I'm not sure I know what you mean.

It seems like JSPM is arbitrarily restricting the importmap too much. Hoping for a simple importmap.

@guybedford
Copy link
Member

@trusktr it's treated more like a compression - if you use lots of things with the same overall rule, you can think of trailing slash mappings as a kind of compression of specific rules. It doesn't align with the underlying usage of the exports field though, it aligns with how the package is used. It takes two imports to get the "compression".

@trusktr
Copy link
Author

trusktr commented Oct 25, 2023

I'm still not understanding. Is there a doc page on this?

Perhaps it would be better if there was a simple mode, and an optimized mode, with a toggle switch in the UI.

The simple mode would just map module names to entry points, or folders, and files would never be map keys. More like a hand-written importmap.

Optimized mode would do the extra stuff that may not always be desirable from a simplicity stand point.

@guybedford
Copy link
Member

Your latest Loom example demonstrates this feature for three pretty well?

@trusktr
Copy link
Author

trusktr commented Nov 3, 2023

Lume* :) (not to be confused with Loom)

I see the Three.js map has the folder alias.

If JSPM can default to folder aliases (or have an option for that), it would be simpler. Then I could change my source code without also having to change the importmap.

In another sense, vanilla ESM has no concept of restricting files you can import, and perhaps JSPM's importmap is synthetically adding this restriction, but I'd like to not have that restriction.

With vanilla ESM, I can easily change this,

import {} from 'https://foo.com/path/to/something.js'

to

import {} from 'https://foo.com/path/to/other-thing.js'

but with JSPM import maps, changing this,

import {} from 'foo/path/to/something.js'

to

import {} from 'foo/path/to/other-thing.js'

could throw an error, which can be annoying when we know that foo/path/to/other-thing.js exists.

I just want as little as possible in the way, in both tooling and configuration.

@guybedford
Copy link
Member

guybedford commented Nov 4, 2023

The package.json exports field permits lots of mapping rules, some of which are similar to folder mappings, and some of which are not. To provide the correct package mappings with the exports field during import map generation, we need to generate mappings that check the exports field for every single map variation. Therefore folder mappings are not something that can be integrated early into the process, but must be treated as a rewriting at the end of import map generation.

When doing folder mapping rewriting it is - (1) only applied to scopes and not top-level imports and (2) only applied in scopes when there is more than one mapping in the folder.

For (1) the reason for this is that the import map top-level imports in JSPM act somewhat like a package.json file in providing the list of things you've explicitly "installed". Just like tree-shaking can do optimizations when it knows what you're exporting (so that import * as m from 'x' can be less preferable to import { name } from 'x', since in the explicit cases bundlers have more information), so too with import maps there are benefits to having more information about the top-level imports, allowing them to be an explicit list of entry points from which "map shaking" can ocur.

For (2), the reason we only do reductions when it applies to more than one mapping in a scope is something we could potentially tweak in the import-map package (this is done by the combineSubpaths routine here - https://github.com/jspm/import-map/blob/main/src/map.ts#L178). We could make it so anytime there is an x/x.js: z/x.js that we replace that mapping with x/: z/, but I don't see any major reasons to do this.

I think the use case you're probably looking for is more like (1), but that misses that the concept of imports being enumerating of entry points which provides benefits to import map "shaking".

@trusktr
Copy link
Author

trusktr commented Nov 7, 2023

I haven't heard of import map shaking. What is shaken away? Is there a runtime improvement?

@guybedford
Copy link
Member

If we generated the full import map for all dependencies, it would be very large. We only generate the minimal import map necessary to load a given application. While import maps are small they are also on the critical loading path bottleneck.

@trusktr
Copy link
Author

trusktr commented Nov 14, 2023

If we generated the full import map for all dependencies, it would be very large. We only generate the minimal import map necessary to load a given application. While import maps are small they are also on the critical loading path bottleneck.

Ah ok.

What I suggested leads to a smaller import map. The second example in the OP is smaller than the first example in the OP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants