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

resolvePackageJsonExports doesn't actually work #54574

Closed
ekwoka opened this issue Jun 8, 2023 · 11 comments
Closed

resolvePackageJsonExports doesn't actually work #54574

ekwoka opened this issue Jun 8, 2023 · 11 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@ekwoka
Copy link

ekwoka commented Jun 8, 2023

Bug Report

Using node module resolution doesn't examine exports paths.

adding resolvePackageJsonExports: true is accepted in the tsconfig, but doesn't actually work. It still says ts can't resolve it.

🔎 Search Terms

resolvePackageJsonExports

🕗 Version & Regression Information

Latest

⏯ Playground Link

Can't use npm imports in the playground.

💻 Code

import UI from "@newthink/ui";
{
  "include": ["src/**/*"],
  "exclude": ["src/**/*.spec.ts", "src/**/*.test.ts"],
  "compilerOptions": {
    "baseUrl": "./src",
    "paths": {
      "@/*": ["*"]
    },
    "noEmit": true,
    "declaration": true,
    "outDir": "dist",
    "declarationMap": true,
    "target": "es2017",
    "module": "esnext",
    "moduleResolution": "node",
    "resolvePackageJsonExports": true,
    "noImplicitAny": true
  }
}

🙁 Actual behavior

Cannot find module '@newthink/ui' or its corresponding type declarations.

🙂 Expected behavior

Actually Work

@Andarist
Copy link
Contributor

Andarist commented Jun 8, 2023

Can't use npm imports in the playground.

You actually can, sort of, do that:
TS playground

Note - please reorder exports.{import,types} to exports.{types,import}. In this case the types condition isn't strictly needed anyway because those files have the same names (just different extensions) so you can rely on the sibling lookup. However, if you want to include the explicit types condition it's best to put it first. package.json#exports are actually order-sensitive

@ekwoka
Copy link
Author

ekwoka commented Jun 9, 2023

Okay, I am seeing that in the docs

But if types isn't necessary, since it's index.js and index.d.ts then it also should still work even if the order is wrong.

@Andarist
Copy link
Contributor

Andarist commented Jun 9, 2023

But if types isn't necessary, since it's index.js and index.d.ts then it also should still work even if the order is wrong.

Yeah, it works right now but might be misleading to users or copy-pasted to other projects where the situation will be different.

@fatcerberus
Copy link

@Andarist While I understand the order-dependence, does it matter for types? import and types are, I thought, completely orthogonal (assuming both exist) so it shouldn’t matter which one has “precedence”.

@Andarist
Copy link
Contributor

Andarist commented Jun 9, 2023

While this might feel odd at first - yes, types shouldn't be treated in any special way here.

What should resolve here and how exactly the algorithm should be written to make the rules clear if the overarching rule of package.json#exports is that they are order-dependent?

{
  "exports": {
    "import": {
      "types": "./foo.d.ts"
      "default": "./bar.js"
    },
    "types": "./baz.d.ts"
  }
}

Of course, the outer types could be used here merely as a "fallback" (if the inner one wouldn't have types condition) but imho that only makes things more muddy when that overarching rule is in place.

Currently, it actually might be used as a fallback but that's considered a bug: #50762

@fatcerberus
Copy link

fatcerberus commented Jun 9, 2023

fwiw I was referring more to how import and types relate to each other when at the same level of nesting, and both refer directly to files. I guess you’re implying that if the import comes first then the “expected” behavior is that TS would always use sibling lookup and therefore they aren’t completely orthogonal.

@Andarist
Copy link
Contributor

Andarist commented Jun 9, 2023

fwiw I was referring more to how import and types relate to each other when at the same level of nesting, and both refer directly to files.

Yeah, but what you describe goes into "more rules" territory whereas the basic rule of order-dependency is pretty simple and using it uniformly helps educating the community at large about this fact. Diverging from it introduces a dangerous precedence and creates more complexity for other tools that try to replicate TS behavior when analyzing packages etc. In every other context those keys are order-dependent so treating types as not fully order-dependent would force other implementers to specialcase this .

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Jun 12, 2023
@RyanCavanaugh
Copy link
Member

I'm not seeing a bug described here.

Generally we don't ship flags that are completely broken out of the gate?

@JoostK
Copy link
Contributor

JoostK commented Jun 12, 2023

I think the confusing comes from the fact that this flag only works under Node16, NodeNext or Bundler module resolution:

export function getResolvePackageJsonExports(compilerOptions: CompilerOptions) {
const moduleResolution = getEmitModuleResolutionKind(compilerOptions);
if (!moduleResolutionSupportsPackageJsonExportsAndImports(moduleResolution)) {
return false;
}
if (compilerOptions.resolvePackageJsonExports !== undefined) {
return compilerOptions.resolvePackageJsonExports;
}
switch (moduleResolution) {
case ModuleResolutionKind.Node16:
case ModuleResolutionKind.NodeNext:
case ModuleResolutionKind.Bundler:
return true;
}
return false;
}

That does look unintentional given the switch case, as moduleResolutionSupportsPackageJsonExportsAndImports is only true for its exact cases.

@ekwoka
Copy link
Author

ekwoka commented Jun 13, 2023

It just doesn't seem to do anything. Or the spec doesn't clarify when it should or shouldn't work.

Basically, the goal would be getting a situation where you can leave off the .js extension, and use packages that don't have main or types entries in the package JSON, using exports only. None of the resolution options allow that.

Based on the docs, the aforementioned flag seems like it should use exports always regardless of other settings. But that's not the case.

Also, limiting it to those mentioned modules resolutions also means it's not needed, no? Since those already use exports.

@typescript-bot
Copy link
Collaborator

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

6 participants