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

TS 5 Regression. tsconfig extends stop resolving references to node_modules #53314

Closed
Vidda777 opened this issue Mar 17, 2023 · 30 comments · Fixed by #53443
Closed

TS 5 Regression. tsconfig extends stop resolving references to node_modules #53314

Vidda777 opened this issue Mar 17, 2023 · 30 comments · Fixed by #53443
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@Vidda777
Copy link

Vidda777 commented Mar 17, 2023

Bug Report

🔎 Search Terms

TS 5, tsconfig, extends, node modules

🕗 Version & Regression Information

5.0.2

In latest stable build, 4.9.5, tsconfig extends resolve correctly references to libraries on node modules. However, after trying out 5.0.2 it has stop working

  • This changed between versions 4.9.5 and 5.0.2

⏯ Playground Link

No Playground, I quickly check and there is no support for tsconfig, package.json,...

💻 Code

No code, it's just configuration. Having a package.json declaring

 "devDependencies": {
    "@mylibrary/commons": "1.0.0",
    "typescript": "5.0.2"
  }

and a tsconfig with:
"extends": "@mylibrary/commons/configs/tsconfig.backend.json",

🙁 Actual behavior

The command tsc --build finish with error:

tsconfig.json:2:14 - error TS6053: File '@mylibrary/commons/configs/tsconfig.backend.json' not found.

2   "extends": "@mylibrary/commons/configs/tsconfig.backend.json",

🙂 Expected behavior

The command tsc --build finish correctly.

@ilogico
Copy link

ilogico commented Mar 17, 2023

This is most likely because @mylibrary/commons has an exports field on package,json without exporting the file.

@jakebailey
Copy link
Member

I don't think we consider exports for tsconfig, just paths. But, I could be wrong.

I think what we need is the layout of whatever @mylibrary/commons is (plus maybe package.json). Bascially, a minimal repro, as this issue isn't enough.

@ilogico
Copy link

ilogico commented Mar 17, 2023

The reason I said is because I hit this problem when upgrading to 5.0 and I fixed it by adding an entry to the exports map.

@jakebailey
Copy link
Member

Oh, very interesting!

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Mar 17, 2023
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.1.0 milestone Mar 17, 2023
@RyanCavanaugh
Copy link
Member

I think this is a bug, AFAIK extends lookups should always be done as CJS lookups. Probably we'll have to revisit that logic at some point...

@ilogico
Copy link

ilogico commented Mar 17, 2023

Subpath exports are not specific to ESM.
In fact, I would say ignoring exports was the bug, even if fixing it is a breaking change.

@ilogico
Copy link

ilogico commented Mar 17, 2023

And considering this to be a bug and "fixing" it is also a breaking change.

@RyanCavanaugh
Copy link
Member

"breaking change" does not include undoing accidental changes back to the prior behavior. That's an unusable definition.

@ilogico
Copy link

ilogico commented Mar 17, 2023

I don't want to discuss the semantics of breaking changes. Of course I accept that fixing bug might be a desirable breaking change, that was my initial point.
I think it's reasonable to assume that TS would use the same lookup algorithm as NodeJS and that everything else is a bug, a quirk, non-standard behaviour or whatever you want to call it. But if you think it's more important to keep the previous behaviour, I would say that's reasonable as well.

But maybe there's a middle ground where the lookup is done in the standard way (looking at the exports field) and then falling back to the directory structure as it did before.

@loucyx
Copy link

loucyx commented Mar 18, 2023

IDK if this is useful for the conversation and for debugging, but I had my exports field with this property set for a while now:

"exports": {
  // ...
  "./*.json": "./lib/*.json"
  // ...
}

And I still get the "File not found" error when trying to import it from other packages, which indeed is a regression from my point of view, but also indicates that the fix isn't "just using exports," because the "*" syntax is valid, and also having to specify file by file in the exports field is kinda unrealistic.

Hope I'm adding something useful to the conversation and this isn't just unwanted noise.

@mike-lischke
Copy link

mike-lischke commented Mar 18, 2023

I just upgraded to TS 5 and found the same issue with one of my own node packages ( repo, npm). With TS 5 I now get the error

Cannot find module 'antlr4-c3' or its corresponding type declarations.ts(2307)

Needless to say that I did not change any bit in the project where I'm using the node package. Just upgrading to TS 5 broke the import.

I played a bit with other node module resolution settings, but nothing helped. What sticks out when looking in the bundled node package is that there's no index.js file in the root folder. All the code is in the out/ folder.

Update: I solved the issue by setting the main entry in package.json to ./out, so it's not a critical problem (for me).

@jakebailey
Copy link
Member

@mike-lischke This issue is about tsconfig's extends clause. Your issue is unrelated; please file a separate issue.

@Vidda777
Copy link
Author

Hi, sorry for not answering sooner. What @ilogico said seems reasonable as a workaround, adding the config file as an export entry in the package.json of the library. We don't have it there and maybe this is the way to enforce it to be "exportable". Thanks for the tip.

However, the current TS 5.0.2 behaviour is a regression to what previous 4.x did. So here's the point, as I didn't see a 5.0 changelog announcing this as a breaking change (and the corresponding how-to fix), I assume this is a legit regression bug.

Thanks to all who have contributed to the post. Sorry if I didn't add a fully reproducible environment, I though it was straighforward case.

Anyway, here's the part of the package.json and the capture of the library structure

"exports": {
    ".": {
      "import": "./dist/mjs/src/main/index.js",
      "require": "./dist/cjs/src/main/index.js"
    },
    "./configs/jest.config.js": {
      "require": "./configs/jest.config.js"
    },
    "./configs/.eslintrc.backend.js": {
      "require": "./configs/.eslintrc.backend.js"
    },
    "./configs/.prettierrc.js": {
      "require": "./configs/.prettierrc.js"
    }
  },

image

@jakebailey
Copy link
Member

I assume this is a legit regression bug.

Yes, note the label on the issue.

@andrewbranch
Copy link
Member

It looks like the current behavior was accepted as being intentional in #48665, and implemented in #50955. Are we wanting to reconsider that decision, or did we just forget to mark it as a breaking change?

@loucyx
Copy link

loucyx commented Mar 20, 2023

Even if it is intentional, is still "broken", because as I mentioned in my previous comment, the "*" exports don't work. So #50955 doesn't fully support the exports specification. It's either a regression or an incomplete breaking change.

@andrewbranch
Copy link
Member

Yeah, it looks like the exports pattern trailer feature flag is just missing.

@andrewbranch
Copy link
Member

We decided this was intentional, minus the wildcard issue, which I’ll put up a fix for shortly. If a package wants to make its tsconfig.json available for extending, and has an "exports" in its package.json, it needs to make sure the tsconfig is reachable via those "exports".

@barca-reddit
Copy link

We decided this was intentional, minus the wildcard issue, which I’ll put up a fix for shortly. If a package wants to make its tsconfig.json available for extending, and has an "exports" in its package.json, it needs to make sure the tsconfig is reachable via those "exports".

Hello. Apologies if this is a silly question, but is this fix for wildcard exports going to be part of Typescript 5.0.3? I checked the milestones page for 5.0.3 and 5.1.0 and it wasn't part of it.

I also tested the fix with typescript@next, but I was still getting the same error as before:

// package.json
{
    "name": "@repo/config",
    "exports": {
        "./*.json": {
            "require": "./typescript/*.json"
        }
    }
}

// tsconfig.json (elsewhere in the monorepo)
{
    "extends": "@repo/config/typescript/tsconfig.browser.json",
}
// ❌ File '@repo/config/typescript/tsconfig.browser.json' not found.

Essentially all I am asking is when is this going to be available in nightly or main versions of TS?

Cheers!

@jakebailey
Copy link
Member

The fix is already available in nightly. I just had the bot open a PR to pull the fix back into 5.0, since I think we wanted that.

If it's not working in nightly, then that's a problem.

@barca-reddit
Copy link

If it's not working in nightly, then that's a problem.

Apologies for the false alarm. After spending some more time to setup everything from scratch on a clean repository, I got the nightly version to work correctly with wildcards, so the fix is indeed working.

I just had the bot open a PR to pull the fix back into 5.0, since I think we wanted that.

Just one quick question if you don't mind - if I understand correctly, this means that when the next version of Typescript releases (5.0.3), this fix will be included in it, and until then I can use nightly versions?

Cheers!

@jakebailey
Copy link
Member

If we take it, 5.0.3.

@barca-reddit
Copy link

If we take it, 5.0.3.

I am not entirely certain what to "take it" means, but nevertheless I appreciate the response and your help. Will just be keeping an eye on the release notes.

Cheers again! 👍

@jakebailey
Copy link
Member

See #53557. @DanielRosenwasser

"If" was a little too loose; we definitely wanted this to be backported to 5.0, per design meeting discussion. We just have to merge the cherry-pick before release.

achingbrain added a commit to ipfs/aegir that referenced this issue May 17, 2023
TypeScript used to be able to resolve the extends field in `tsconfig.json`
using the node require algorithm.

Recent versions of TypeScript have a regression that means that if
a module has an exports map, the extendable typescript config file
has to be in that exports map so add it here.

Refs: microsoft/TypeScript#53314
achingbrain added a commit to ipfs/aegir that referenced this issue May 17, 2023
TypeScript used to be able to resolve the extends field in `tsconfig.json`
using the node require algorithm.

Recent versions of TypeScript have a regression that means that if
a module has an exports map, the extendable typescript config file
has to be in that exports map so add it here.

Refs: microsoft/TypeScript#53314
@naftalimurgor
Copy link

Still experiencing the same issue. Was this fully resolved?

@kirillgroshkov
Copy link

Still experiencing the same issue. Was this fully resolved?

Yes, we also noticed the regression in 5.3.2 (while it was working in 5.2.2).

@jakebailey
Copy link
Member

The original issue here is most definitely fixed. If you are having a problem in a recent release, please file a new issue.

If you are using ts-node, maybe you are seeing #56492.

@kirillgroshkov
Copy link

If you are using ts-node, maybe you are seeing #56492.

It turned out to be exactly that, thank you for the pointer 🙏

@milon27
Copy link

milon27 commented Jun 13, 2024

yes facing the same issue

@microsoft microsoft locked as resolved and limited conversation to collaborators Jun 13, 2024
@RyanCavanaugh
Copy link
Member

Please open a new issue with repro steps if you're having a problem

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.