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

Specify import_map.json directly in deno.json #15816

Closed
lcswillems opened this issue Sep 8, 2022 · 12 comments · Fixed by #17478
Closed

Specify import_map.json directly in deno.json #15816

lcswillems opened this issue Sep 8, 2022 · 12 comments · Fixed by #17478
Labels
suggestion suggestions for new features (yet to be agreed)

Comments

@lcswillems
Copy link

Currently, I have to create a deno.json and a import_map.json file. It would be great if import_map.json content could be moved to deno.json. This way I will now have only one remaining config file.

@dsherret dsherret added the suggestion suggestions for new features (yet to be agreed) label Sep 8, 2022
@dsherret
Copy link
Member

dsherret commented Sep 8, 2022

More discussion on this in the closed issue: #12800

@lcswillems
Copy link
Author

I read the issue but don't understand why what I am proposing would be a bad idea

@dsherret
Copy link
Member

dsherret commented Sep 8, 2022

I'm not suggesting it's a bad idea. I'm just saying that there was more discussion on that in the closed issue.

@lcswillems
Copy link
Author

I wasn't meaning you said it is a bad idea. I rather wanted to say: by reading the issue, I can't figure out what were the reasons they discarded this idea.

@dsherret
Copy link
Member

dsherret commented Sep 9, 2022

Sorry I misunderstood. The issue was closed because it was moreso the tracking issue for adding just an "importMap" key to the configuration file for referencing an import map file, which was implemented.

Originally the semantics included support for the top level "imports" and "scopes" keys in the configuration file. This has been removed from this proposal and can be revisited in the future.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 10, 2022

And this issue is marked as a suggestion, ergo revisiting it.

There are a few challenges though:

  • deno.json and deno.jsonc are always parsed as JSONC (like tsconfig.json is). We want to parse an import map in a spec compliant way. We could relax the rules for Deno, but it would allow users to inadvertently author non-spec compliant import maps.
  • We start to run the serious risk of collisions of keys. Specifically TypeScript is going to tackle import maps at some point in the future, and it would be good to ensure that, like deno.json and deno.jsonc are compatible with tsconfig.json like they are now.

So we abandoned that part of the proposal to ship something we were 99% sure wouldn't cause problems down the road, and held off doing the things that we might regret, as the improvement gain didn't seem to outweigh the risks.

@lcswillems
Copy link
Author

I see! Thank you for having taken the time to explain the situation.

I am not going to present a satisfying solution as it is breaking but what about prefixing the keywords with the name of the tool.

For example:

"importMap": "..." for Deno

and

"ts:importMap": "..." for TS

But for consistency some other keywords should be prefixed so this would be breaking.

@dsherret
Copy link
Member

We are planning on adding "imports" and "scopes" from import maps to the deno.json/jsonc config file's schema.

{
  "tasks": {
    "build:npm": "deno run -A ./scripts/build_npm.ts"
  },
  "imports": {
    "chalk": "npm:chalk@^1.0",
    "cowsay": "npm:chalk@^1.0",
    "std/": "https://deno.land/[email protected]/",
  }
}

In a sense, this would work similar to how html files support embedding import maps. If this is done, the deno.json file is not intended to be used as an import map (it's the deno.json schema adding these keys and not the import map schema). If you wish to do that, then it's recommended to keep the import map file separate as previously done and specify it in a deno config file (ex. "importMap": "./import_map.json").

@lcswillems
Copy link
Author

@dsherret That's seems interesting! I am not sure to understand why with this addition I will still need to use an import map file?

@dsherret
Copy link
Member

Sorry, I wasn't clear. This addition doesn't require you to still use an import map file. That's only suggested if you want to share the import map with other tools.

@lcswillems
Copy link
Author

I see! So that's very great news!!!

@aapoalas
Copy link
Collaborator

aapoalas commented Jan 22, 2023

Would it not make more sense to simply "overload" the importMap key's schema to be either a string (reference to JSON file), or an inline import map? That would also somewhat future-proof this method, as key clashes would be avoided and additionally if import maps become composable through eg. defining multiple ones in an array (like how System.js does it already) then the importMap key could still be used similarly to define an array of import map objects. With imports and scopes its unclear how composing could be done in the future.

The main-level imports object also calls strongly back to package.json's dependencies which is an understandable thing but it might be better to just use dependencies if the intention is to be similar to NPM.

EDIT: Overloading the importMap property would also keep it clear that it's not possible to define both imports / scopes AND importMap, since import maps are not composable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants