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

Sugary defaults: using a string instead of just the package object #4

Closed
domenic opened this issue Mar 15, 2018 · 7 comments
Closed

Comments

@domenic
Copy link
Collaborator

domenic commented Mar 15, 2018

As noted in one of the examples, we could allow shortening of { main: "moment.js" } to just "moment.js". Then you could write package map files like:

{
  "path_prefix": "/node_modules/",
  "packages": {
    "moment": "moment.js",
    "html-to-text": "index.js",
    "redux": "lib/index.js"
  }
}

which is appealing for its simplicity. (Note how we still assume there's a path segment/folder corresponding to the package name, so e.g. redux is located at /node_modules/redux/lib/index.js. That follows from the default value for "path".)

This slightly complicates the data model, as it would be the first instance of a union type in our structure. But it doesn't seem like that big of a deal. Should we do it?

@justinfagnani
Copy link
Collaborator

This seems like a very nice ergonomics win for handwritten maps.

It's probably worth noting somewhere that package name maps are very similar to loader configs that have been around for a long time in loaders like RequireJS, etc. I think most of them have a simple path mapping like this.

@guybedford
Copy link
Collaborator

I like this roughly in principle, but I would suggest not assuming the package name is in the path prefix. For example, packages can be installed under custom names in package.json files in npm.

Also consider users writing this sugar case by hand to point to CDN resources, and being confused when everything gets prefixed with the package name when they're providing a direct relative URL from the prefix base.

As mentioned in #3 (comment) I'd prefer to be explicit, and also error when things are underspecified by distinguishing between file and directory cases, and treating this shorthand itself as a specific "file package" case and not a sugar.

@domenic
Copy link
Collaborator Author

domenic commented Mar 15, 2018

@guybedford we already are assuming the package name is the default path per https://github.com/domenic/package-name-maps#using-path_prefix-and-the-default-path . If you think we should remove that default (note: not a hard assumption, just a default) then perhaps open another issue.

I don't think we have any intention of supporting "file packages" in the current design. Remember, this is a package name map, not a module name map.

@guybedford
Copy link
Collaborator

guybedford commented Mar 15, 2018

The path default is fine, but what I'm suggesting is that this sugar case could make it easy to handle a default case where users know where their modules are and just want to direct the packages to them.

For example if users have a few packages they want to load from a CDN and want to just write a map like:

{
  "path_prefix": "https://cdn.com/"
  "packages": {
    "moment": "[email protected]/index.js"
    "lodash": "[email protected]/dist/lodash.js"
  }
}

Requiring the above to need an object with both a "path" and a "main" field seems unfortunate otherwise for users writing these configurations by hand.

@dcleao
Copy link

dcleao commented May 26, 2018

@domenic wrote:

I don't think we have any intention of supporting "file packages" in the current design. Remember, this is a package name map, not a module name map.

Why is that? Why not?
I had asked myself on that aspect. While a package map solves the CommonJS case, it only partially solves the RequireJS/AMD case. It doesn't look like a big stretch.

Apart from the packaging concept, and the aliasing that the main module allows, aliasing is a useful feature by itself.

As it is, I can see people defining sub-packages and the main attribute to achieve aliasing, any way. I think the design only impedes the top-level alias to a file.

While the current proposal is already a breath of fresh air, I would like to see you clearly helping out the people who have large investments in AMD and that wished one day they could evolve and leave AMD behind.

@guybedford
Copy link
Collaborator

It seems I misunderstood the resolution rules here slightly. In my previous example, the alternative working with this approach would be to write:

{
  "path_prefix": "https://cdn.com/"
  "packages": {
    "moment": "/[email protected]/index.js"
    "lodash": "/[email protected]/dist/lodash.js"
  }
}

which seems perfectly fine. So I'm definitely for this then.

guybedford added a commit to guybedford/import-maps that referenced this issue Jun 27, 2018
guybedford added a commit to guybedford/import-maps that referenced this issue Jun 28, 2018
domenic pushed a commit that referenced this issue Jun 28, 2018
This adds the package main sugar case from #4, as well as more test cases in related areas.
@domenic
Copy link
Collaborator Author

domenic commented Jun 28, 2018

This was added to the tests and reference implementation in #44. I suppose we should update the README too before closing this issue.

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

4 participants