Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Fix typeRoots for tsconfig.json #5956

Merged
merged 1 commit into from
Mar 14, 2023
Merged

Fix typeRoots for tsconfig.json #5956

merged 1 commit into from
Mar 14, 2023

Conversation

sheetalkamat
Copy link
Contributor

@sheetalkamat sheetalkamat commented Mar 8, 2023

PR description

Testing instructions

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if documentation updates are required.

Breaking changes and new features

  • I have added any needed breaking-change and new-feature labels for the appropriate packages.

Fixes build breaks with typescript@next per microsoft/TypeScript#53122 (comment) because of incorrect typeRoots

@gnidan
Copy link
Contributor

gnidan commented Mar 8, 2023

Thanks for this! We've intentionally listed the types explicitly (rather than the whole node_modules/@types folder) because it's caused problems in the past, but we'll see what our CI says about this now.

@sheetalkamat
Copy link
Contributor Author

Specifying typeroot as "../../node_modules/@types/node", does not include node setting types: ["node"] does that which you have configured correctly. The typeroot should just be "node_modules" or "node_modules/@types" instead.

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

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

This matches typeRoots documentation, typescript compiles and LSP continues to serve meta. Thanks @sheetalkamat !

@gnidan
Copy link
Contributor

gnidan commented Mar 8, 2023

Specifying typeroot as "../../node_modules/@types/node", does not include node setting types: ["node"] does that which you have configured correctly. The typeroot should just be "node_modules" or "node_modules/@types" instead.

Thanks for sharing your understanding!

I'm specifically concerned about certain packages needing to exclude some types. For example, @truffle/dashboard includes types for DOM-related things, but other packages would barf if those types got included... and because this is a monorepo, packages were inheriting these browser-land types from <repo-root>/node_modules/@types and tsc would attempt to compile them even though they weren't relevant.

I thought typeRoots was the way to ensure we could control these type exclusions, by making everything include-only. This isn't the case?

(I'll have to go through the package.json files again to refresh myself, I guess. Also, if CI says we're good, then seems like no problem :)

@cds-amal
Copy link
Member

cds-amal commented Mar 9, 2023

I thought typeRoots was the way to ensure we could control these type exclusions, by making everything include-only. This isn't the case?

It's possible that version upgrades since 2019 may have changed some assumptions.

$ git log  -S"@types/mocha"
Pickaxe output
Tue Mar 7 16:24:54 2023 -0800 261cbd757 (HEAD -> sheetalkamat/develop) Fix typeRoots for tsconfig.json  [Sheetal Nandi]
Tue Sep 27 11:49:50 2022 -0400 ed96b8dab continue typing lazily to get test successfully building fast  [eggplantzzz]
Wed Jul 27 17:17:58 2022 +1200 2df562a36 Standardize @types/node for v12 LTS  [Ben Burns]
Fri Jul 29 08:12:45 2022 -0400 23610ab28 Revert "Bump typescript to 4.7.2 and @types/node to 18.6.1"  [cds-amal]
Wed Jul 27 17:17:58 2022 +1200 ba3f0164c Use latest node types, remove unused typeRoots  [Ben Burns]
Wed Jun 22 13:04:07 2022 -0400 6820f3898 feat: Add initial TS support for migrations  [Ben Burns]
Wed Mar 23 16:49:21 2022 +1300 4f8a46974 add package for handling multiple spinners  [Ben Burns]
Thu Jul 15 17:22:54 2021 +0200 379d017a6 Rename browser-provider-server to dashbaord-message-bus  [Rosco Kalis]
Wed Nov 17 15:32:00 2021 -0600 fed63c52f remove package-lock.json  [leeftk]
Fri Nov 12 10:10:47 2021 -0600 1ec168e8c update yarn.lock  [leeftk]
Fri Oct 29 10:47:44 2021 -0400 89f047b2e add test for fetch-and-compile  [Lee Faria]
Thu Sep 16 12:07:15 2021 -0400 798a7cdc3 Remove unnecessary deps from @truffle/profiler  [eggplantzzz]
Thu Sep 16 11:18:25 2021 -0400 4c0e07845 Move profiler out of compile-common into its own package  [eggplantzzz]
Fri Feb 5 01:48:02 2021 -0500 74c9be849 Update dev dependencies to consolidate versions  [Harry Altman]
Mon Aug 24 13:29:57 2020 -0400 38d45e59d Move some packages away from using the shims in compile-solidity/legacy and on to compile-common  [eggplantzzz]
Mon Apr 13 21:57:10 2020 -0400 f0d9e2a54 Convert @truffle/resolver to TypeScript  [g. nicholas d'andrea]
Mon Nov 18 14:04:26 2019 -0800 985497feb yarn add -D @types/mocha (code-utils)  [CruzMolina]
Fri Nov 15 16:12:01 2019 -0800 2ea1b3726 add missing mocha types [@types/mocha] (blockchain-utils)  [CruzMolina]
Wed Sep 25 18:11:18 2019 +0300 bb3b19e3d @truffle/config migrated to typescript  [Vladislav Tupikin]
Tue Sep 24 18:47:59 2019 +0300 343648d1f refactor: migration to typescript  [Vladislav Tupikin]
Tue Jul 23 11:30:17 2019 -0700 c86fe57ed add tsconfig  [CruzMolina]
Mon Jul 22 14:23:55 2019 -0700 d46c27c00 add tsconfig  [CruzMolina]
Wed Jul 17 14:47:15 2019 -0700 3ef248c7c add tsconfig  [CruzMolina]
Thu Jul 11 15:46:55 2019 -0700 ae0fc47c8 add tsconfig  [CruzMolina]
Mon Apr 29 23:24:31 2019 -0400 b9545c42e Update tsconfigs to specify explicit @types pkgs  [g. nicholas d'andrea]
Mon Apr 29 23:24:31 2019 -0400 3a0a5370e Update tsconfigs to specify explicit @types pkgs  [g. nicholas d'andrea]
Mon Apr 22 13:39:47 2019 -0400 dbfe644f6 port the interface-adapter to typescript  [Mike Seese]%

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

Thanks again for opening this!

@gnidan gnidan merged commit 542afa9 into trufflesuite:develop Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants