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

Relax package.json#imports prefix #49182

Closed
bmeck opened this issue Aug 15, 2023 · 31 comments
Closed

Relax package.json#imports prefix #49182

bmeck opened this issue Aug 15, 2023 · 31 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem. stale

Comments

@bmeck
Copy link
Member

bmeck commented Aug 15, 2023

What is the problem this feature will solve?

Tools such as rspack, bun, typescript, remix, and turbo are all looking at using tsconfig.json#compilerOptions.paths . This is in part due to constraints on package.json#imports and it lacking the ability to fully alias the entire import space. These also seem to have differing constraints and some are doing something outside the standard TS behavior on purpose.

What is the feature you are proposing to solve the problem?

The package.json#imports field seems sufficient from reading some twitter threads involving a few maintainers from remix or rspack except for the prefix # constraint. I propose getting feedback from these tools on removing the constraint if that would be sufficient for their users’ general needs. If it is sufficient we could encourage people to use a single way of doing this allowing tooling to avoid divergence and interop woes.

This would not cover global aliasing which already can be covered with policy redirects but likely is not desirable/sufficient for some subset of users. My hope is that subset is small enough to avoid the issue blocking this entirely.

What alternatives have you considered?

Supporting tsconfig.json in a limited subset to alleviate the user needs.

cc @nodejs/loaders @nodejs/modules

@bmeck bmeck added the feature request Issues that request new features to be added to Node.js. label Aug 15, 2023
@hardfist
Copy link
Contributor

hardfist commented Aug 15, 2023

As a maintainer of Rspack, I find our users(webpack | rspack users) choose tsconfig paths over subpaths imports for the following reasons:

  • education: it seems subpaths imports are little known, very few materials talking about this features(not everyone like reading NodeJS docs carefully especially it's a little section of docs)
  • typescript support: people perfer tsconfig paths other than config webpack resolve.alias manually because it's the single source of truth about alias, it both affects vscode intelligence and bundle alias behavior
  • migration cost: https://github.com/dividab/tsconfig-paths-webpack-plugin is well known and most of users choose custom alias prefix like ~/components and @/components and since subpaths force choose # as alias prefix, it will cause large migration cost especially for large monorepo
  • monorepo support: there're some wepack plugins supports nest alias and tsconfig paths itself supports composite mode, so you can use different alias for different package
  • multi tools support: both vite、webpack、bun、esbuild supports tsconfig paths( either out of box or by well known plugin)

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

@hardfist

I think the main goal of removing the prefix would be to reduce the migration costs here. You could still use whatever prefix you desire such as ~ or @ if we remove the current self imposed constraint of always requiring a # prefix for local aliasing.

An additional desire might be to move a single source of truth since TS and bundlers also support tsconfig in various plugin/etc scenarios but the runtime does not. Making runtime and bundler behave the same is a big maintenance burden quite often. I'm open to alternative that solve this but it seems removing the prefix would solve this use case including monorepos at the cost of a tooling update.

@hardfist
Copy link
Contributor

TBH tsconfig paths is not an ideal way to treat as single source of truth because it's super complex and the parsing work is very high(json comment support, extends support, project reference support), but since it's the only alias configuration typescript recognize, I can't think of a better alternative, maybe @andrewbranch have better suggestions?

@Andarist
Copy link
Contributor

Personally, I very much like that there is an enforced prefix there. It makes it obvious what this import specifier is about and it also clearly defines where I should look for the "manifest" of how this specifier might be resolved.

To address some of the @hardfist's points:

  • education: ye, this is a problem. Clearly, most people don't realize that package.json#imports is a thing. Tools can encourage the "standard" convention though (over tsconfig paths) and with time people should gain a herd knowledge about this stuff
  • typescript support: package.json#imports are supported by TS just fine. However, you need to enable new moduleResolution modes to opt-into them. This is unfortunate and maybe could somehow be reconsidered. I understand the risk related to enabling package.json#exports support by default (eg. many pkgs are misconfigured when it comes to how they point to types so it would create problems for many, the situation should get better with time). Right now I can't even ship a package with those specifiers used in declaration files because many users won't be able to resolve them when they use moduleResolution: node. Auto-imports don't work for those but there is a PR that aims to fix this: Add auto-import for the package.json imports field microsoft/TypeScript#55015
  • single source of truth: I think this is somewhat misguided. While some tools might handle tsconfig paths automatically, many just won't do that. So once you start considering multiple test runners, lower-level bundlers etc you end up re-configuring tsconfig path aliases all over the place. package.json#imports can actually act as that single source of truth because they are part of the standard~ resolution algorithm. In most cases, it should be enough to configure them for all of the tools to just pick them up.
  • migration cost: sure, there is that. I think though that in the long run it's worth it. The requirement to use this out of the box is using modern tools. And with older tools, the gap can usually be filled in - if you can source those aliases from tsconfig paths then you can "manually" source them from package.json#imports as well. Other than that the cost is to run a quite simple find+replace over the whole codebase.
  • monorepo support: I'd love to hear more about this use case since I'm not sure if I understand what it is about.
  • multi tools support: that's the beauty of package.json#imports though... they are already supported by all of them!

@ljharb
Copy link
Member

ljharb commented Aug 15, 2023

I think the enforced prefix is critically important for clarity.

Aesthetics aren't what's important here, and eventually the long tail of tools that have done things their own way for a decade will catch up to doing what node does, as they've always done, and people will migrate to the built-in approach.

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

@ljharb this isn't purely aesthetics, these are being used to alias bare specifiers like package names which is a different capability than what we currently allow.

@andrewbranch
Copy link

If I had to guess, I would say that the # prefix is not the reason people use tsconfig paths when they could use package.json imports. People are using tsconfig paths because they predate imports by many years and have a long history of bundlers reading them into their own config. The issue isn’t a single character restriction; it’s that any change is burdensome and hard to justify when something isn’t actively broken. If every new bundler supports tsconfig paths out of the box, nobody is going to migrate to package.json imports for their bundled apps, regardless of whether or not they can do so without changing module specifiers in existing source code.

I think things would be marginally better if modern tools didn’t adopt tsconfig paths, but clearly it’s up to each tool’s maintainers to know their users. From my perspective, it seems like there were good reasons to want to avoid reading paths, and for the work it took to implement, they could have implemented a migration tool instead. But I’m not convinced that changing imports will make a meaningful impact toward convincing users to transition off of paths.

I personally like the # prefix because a human reader knows what’s going on at a glance. (Less importantly, I also like the subtle connection to #private class fields.)

@ljharb
Copy link
Member

ljharb commented Aug 15, 2023

@bmeck npm already has overrides for that.

@hardfist
Copy link
Contributor

hardfist commented Aug 15, 2023

from new tool point of view,migration cost is the key point of new tools adoption,if users can migrate to subpath import without changing code,we can sugget users do that,but if they need to modify tons of code,new tool has to find new way to meet users needs to reduce migration cost other than ask users to change their code to meet standard

@GeoffreyBooth GeoffreyBooth added module Issues and PRs related to the module subsystem. esm Issues and PRs related to the ECMAScript Modules implementation. labels Aug 15, 2023
@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Aug 15, 2023

Unless I’m misremembering, the #-as-first-character requirement was to disambiguate from bare specifiers and npm scopes (that start with @). Maybe that doesn’t need to be a concern, that we can lift the restriction and it becomes “try "imports" first and if no match found, then try the rest of the current resolution algorithm” but I think it would be helpful to dig up the notes from when "imports" was first designed to understand why these decisions were made.

There’s also self-referencing a package by its name, which is a different, even less well-known feature. You can do something similar to what you’re discussing above, relative to the root of the project. For example, with a package.json like this:

{
  "name": "~",
  "type": "module",
  "exports": {
    "./*": "./*"
  }
}

and a foo.js like export const num = 3, you could then write:

import { num } from '~/foo.js'

And the num here would be 3. This is because the package.json "name" field is ~, and because we’ve defined the "exports" field.

You could also use the "exports" mapping to do stuff like move the root one level down:

{
  "name": "~",
  "type": "module",
  "exports": {
    "./*": "./src/*"
  }
}

Then if you move foo.js to src/foo.js, you would be able to import it via import { num } from '~/foo.js'.

It doesn’t need to be ~, either; any string that’s valid as a package name will work.

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

@ljharb overrides are quite different, they affect siblings and propagate through the module graph in various ways such as fallthrough. Additionally they are an affect at installation time rather than runtime so things that do work using imports work without an npm install to sync/init the data but not so for overrides which do need to run npm install to sync changes or init.

@GeoffreyBooth to some extent that can solve it but using self referential import w/o a valid registry name means it cannot be used for dependencies/monorepo scenarios. It also lacks one of the key reasons that people are using tsconfig.json#compilerOptions.paths which is the ability to redirect bare imports. This would indeed need to be processed for every module specifier which does slow things down but only within packages with this use case opted-in this check does not need to be applied for places not using # prefix since # would still only be "valid" as a specifier prefix universally for imports matching due to various name restrictions already in place.

@ljharb
Copy link
Member

ljharb commented Aug 15, 2023

It sounds like, rather than removing the prefix, what node may want is application-level import map support?

@GeoffreyBooth
Copy link
Member

using self referential import w/o a valid registry name means it cannot be used for dependencies/monorepo scenarios. It also lacks one of the key reasons that people are using tsconfig.json#compilerOptions.paths which is the ability to redirect bare imports.

When you say “redirect bare imports” are you saying like “I want to redirect 'lodash' to 'lodash-es'“ or like “I want to redirect ~/tests to ~/src/tests? Because the latter can be done with package self-reference and "exports" already (see my example above). The former I think can be done with some other feature like npm package overrides? I forget.

Regardless, I think this is getting too abstract. If tsconfig.json paths have multiple use cases, then it’s too unclear to ask for replicating everything they do as a single feature request. Maybe split it apart into specific use cases with clear examples of how it’s done with tsconfig and what the proposal is for how it could be done in Node (or better yet, whether it is already possible in Node). I’m not sure that Node needs to replicate all the existing use cases of tsconfig paths, either; I have a feeling we already cover the most common needs, and the rest can be handled via the Loaders API or by people just continuing to use tsconfig like they are today. The resolution algorithm and package.json are already very complicated, and I’m not sure that adding even more complexity (and performance cost) is worth it for every use case that people can think of, especially since tsconfig seems to do the job pretty well already for most people.

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

@ljharb no, these are being locally aliased inside a package scope. We can already do global aliasing with policies for administration anyway so that is less needed. This is for package authors.

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

@GeoffreyBooth the ask is pretty small already I am not willing to split it up. Likewise not up for long months of discussion so not eager to go down that route I've done before

@GeoffreyBooth
Copy link
Member

the ask is pretty small already

Not really. The proposed solution about # is small, but the ask is:

Tools such as rspack, bun, typescript, remix, and turbo are all looking at using tsconfig.json#compilerOptions.paths . This is in part due to constraints on package.json#imports and it lacking the ability to fully alias the entire import space. These also seem to have differing constraints and some are doing something outside the standard TS behavior on purpose.

Which is essentially, “cover all use cases that tsconfig.json paths enables” which is huge. I can’t even begin to analyze whether the "imports" # proposal starts to cover those use cases, without some list of them. The first few replies have long lists of use cases for paths, and they seem incomplete. This is just too broad to know what to do with.

You can close it if you’d like, or reopen with a focused use case.

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

Definitely not the ask. The ask is to relax the rule to cover more use cases

@arcanis
Copy link
Contributor

arcanis commented Aug 15, 2023

As a quick note, Yarn supports this pattern natively via the link: and portal: dependency types. For example:

{
  "dependencies": {
    "app": "link:./src"
  }
}

Since those protocols aren't fully supported by other package managers they aren't suitable for published libraries, but they work as you would expect for regular applications.

@GeoffreyBooth
Copy link
Member

Definitely not the ask. The ask is to relax the rule to cover more use cases

What use cases?

@bmeck
Copy link
Member Author

bmeck commented Aug 15, 2023

From the general support of other tools the field in question is used for:

  • aliasing bare names locally e.g. pages/* => ./src/pages/*, this is allows the aliasing to match bare specifiers and invalid specifiers which imports currently bans.
  • aliasing special directories (this is already somewhat covered by imports but isn't in widespread use as discussed above)

These actually very closely match how imports works without conditions per https://www.typescriptlang.org/tsconfig#paths

Importantly:

so paths should only be used to inform TypeScript that another tool has this mapping and will use it at runtime or when bundling.

node currently lacks the ability to actually map all of these things due generally to the # prefix restriction; various capabilities such as the array fallback feature isn't heavily used when reading blog posts/articles or quick searches around twitter.

Similarly, other tooling like @arcanis is pointing out does provide local aliasing options for similar purposes. I do not think the solution needs to match tsconfig less we just want to implement that but a variety of tooling is covering things that imports appears to lack due to the syntax restriction in place.

@GeoffreyBooth
Copy link
Member

  • aliasing special directories (this is already somewhat covered by imports but isn’t in widespread use as discussed above)

What’s an example of this?

@ljharb
Copy link
Member

ljharb commented Aug 15, 2023

@bmeck why do they need to be mapped in situ? meaning, if a package author wants a path to be mappable, why not add it to imports, add the # prefix, and eg map #react to react, and then update it later?

I guess what I'm asking for is, why is avoiding that diff churn worth giving up the amount of resolution information you can reliably infer from a file?

@guybedford
Copy link
Contributor

guybedford commented Aug 16, 2023

I suppose the argument here is that there is currently less awareness of imports than there should be and there is certainly a refactoring cost to use it, whereas if the feature didn't require rewriting code at all, it might be more readily adopted.

So we have to weigh if we think there is a very real risk of fragmentation / lack of adoption, versus making the feature more useful but potentially making it harder to trace mapping rules without context.

I don't know the answer to the above, but we should try to listen to users here?

@GeoffreyBooth
Copy link
Member

I don’t know the answer to the above, but we should try to listen to users here?

Sure, but like I wrote earlier, let’s first dig up the reason for the current design. Why the restriction that imports need to start with #?

@guybedford
Copy link
Contributor

The reasoning was that it creates a strong distinction between normal package dependencies and imports field mappings so there is no conflating of the lookup rules. Allowing imports to shadow normal package dependency lookups introduces ambiguity in the module system where the reason why a import 'react' is failing might now not only be because of "dependencies" and node_modules but also some local package.json "imports". # also alludes to the private nature of it in that these are package local, whereas perhaps users might expect this to apply to dependencies in node_modules as well otherwise - eg if a user added "imports": { "react": "./react.js" } and it works locally, but then wonders why it doesn't for a third-party package importing react.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2023

What are the exact needs of users here? Like, I get that it's something about wanting to blindly remap any specifiers inside their package to whatever they want, but why do they want that?

TS paths, in my experience, are mostly used to get a root-relative bare specifier so they don't have to have variable numbers of ../, but self-import addresses this (for anything in exports, at least). Is there a motivation beyond that?

@kasperisager
Copy link

kasperisager commented Nov 21, 2023

I'll weigh in with my own use case: Transparently supporting a JavaScript runtime that doesn't provide any builtins. Without the #-prefix requirement, I could just slap this in my package.json and call it a day:

{
  "imports": {
    "events": {
      "<runtime>": "<runtime>-events",
      "default": "events"
    }
  }
}

Alas, due to the #-prefix requirement I now also have to:

  1. Go through my module source files and replace require('events') with require('#events')
  2. Document that my module must also support <runtime> so imports of assumed builtins must be prefixed with #.
  3. Make all new contributors, virtually all of which use my module from Node.js, aware of 2. to avoid having to repeat 1. over and over again.

Edit: I just realised I can't even do that with a #-prefix because for some reason Node.js doesn't support mapping to builtins. Fortunately, Node.js seems to ignore what it considers invalid mappings and as the runtime in question supports both prefixless specifiers and mapping to builtins I can do the obvious mapping regardless.

@theoludwig
Copy link
Contributor

All the arguments have already been said, but I would like to emphasize, that in my opinion current behavior of Node.js is correct and useful, nothing need to change, Node.js side.

Using # is better than @ or ~, as :

  • @ is already used for org npm packages.
  • ~ is for the $HOME directory on UNIX systems (I don't even know if imports paths supports absolute path, but when I see ~/components it doesn't feel right, and seems to search things in my $HOME directory, even if I know it doesn't).

However, I wish TypeScript, could support it better, but seems like microsoft/TypeScript#55015 already doing it (hope it will be merged soon).
Also, could be cool to have more materials (blogs, video, learning resources), promoting Node.js imports, as it is the "native way" of the runtime, but that's users/ecosystem of Node.js role to do that, not Node.js itself.
I migrated a repo from using lot of ../../ imports, last week: leon-ai/leon-cli@e0ca31b
And will start migrate more of them when needed, in the future.

@firxworx
Copy link

firxworx commented Dec 1, 2023

Reported an issue related to subpath imports and require(): #51009

Copy link
Contributor

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label May 30, 2024
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem. stale
Projects
None yet
Development

No branches or pull requests