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

Updates to type reference directive resolution and module resolution when failed #51715

Merged
merged 9 commits into from
Mar 1, 2023

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Dec 1, 2022

Fixes #37708, #39691, #37958, #44270

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is technically a breaking change, we should discuss whether we think we can get it in 5.0 or wait until 5.1.

@andrewbranch
Copy link
Member

@typescript-bot run dt
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2023

Heya @andrewbranch, I've started to run the parallelized Definitely Typed test suite on this PR at 7d23697. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 13, 2023

Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at 7d23697. You can monitor the build here.

@sandersn
Copy link
Member

@sheetalkamat I'm going through all the ready-to-merge PRs, and this one was intended either for 5.0 or 5.1, so I vote to merge it now that 5.1 development has started.

@sheetalkamat sheetalkamat merged commit 9c5b09c into main Mar 1, 2023
@sheetalkamat sheetalkamat deleted the typeRefUpdates branch March 1, 2023 18:57
sandersn added a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request May 22, 2023
This is a requirement relevant mainly for testing on DT; DT uses
`typeRoots` to redirect packages like react to `@types/react`. But
versions previous to 5.1 had a triple-slash resolution bug when
typeRoots was set [that was only fixed in
5.1](microsoft/TypeScript#51715).

An alternative fix is to move react/experimental.d.ts to
react/experimental/index.d.ts (and the same for react-dom), but I doubt
this bug affects anybody but people who need to change next-pwa on DT,
so I think it's easier to change next-pwa instead.
sandersn added a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request May 22, 2023
This is a requirement relevant mainly for testing on DT; DT uses
`typeRoots` to redirect packages like react to `@types/react`. But
versions previous to 5.1 had a triple-slash resolution bug when
typeRoots was set [that was only fixed in
5.1](microsoft/TypeScript#51715).

An alternative fix is to move react/experimental.d.ts to
react/experimental/index.d.ts (and the same for react-dom), but I doubt
this bug affects anybody but people who need to change next-pwa on DT,
so I think it's easier to change next-pwa instead.
@loick
Copy link

loick commented Jun 2, 2023

Hi there,

Just to notify something on this release. A behavior has changed for me on my monorepo orchestration.
I used to have this:

{
  "extends": "@myapp/config/tsconfig/base.json",
  "compilerOptions": {
    "noEmit": false,
    "outDir": "dist/",
    "types": ["node"],
  },
  "include": ["./src/index.ts"],
  "exclude": ["node_modules", "dist"]
}

Where my app includes in "compilerOptions" :

"typeRoots": ["../../node_modules/@types"]

After this release, I have to specify this typeRoots URL on each package. Is it on purpose?

Thanks 🙏

@ehoogeveen-medweb
Copy link

ehoogeveen-medweb commented Jun 2, 2023

Yes, it's mentioned in the Announcing TypeScript 5.1 blog post.

Edit: Or do you mean that it's not picking it up from @myapp/config/tsconfig/base.json?

@loick
Copy link

loick commented Jun 2, 2023

Yes indeed, sorry maybe it was not clear.

Indeed this is not retrieving the value from @myapp/config/tsconfig/base.json in this case. I already had it before set in my tsconfig/base and it worked properly, but now I have add the typeRoots on every tsconfig file.

@RyanCavanaugh
Copy link
Member

@loick can you log a fresh issue please? Thanks!

Desplandis pushed a commit to Desplandis/DefinitelyTyped that referenced this pull request Jul 3, 2023
This is a requirement relevant mainly for testing on DT; DT uses
`typeRoots` to redirect packages like react to `@types/react`. But
versions previous to 5.1 had a triple-slash resolution bug when
typeRoots was set [that was only fixed in
5.1](microsoft/TypeScript#51715).

An alternative fix is to move react/experimental.d.ts to
react/experimental/index.d.ts (and the same for react-dom), but I doubt
this bug affects anybody but people who need to change next-pwa on DT,
so I think it's easier to change next-pwa instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team Breaking Change Would introduce errors in existing code For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
7 participants