Decrement TypeScript to ~4.6.3 ahead of migration to core monorepo #27
Conversation
- match compiler options with core monorepo - passes all tests Closes #1682
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
I'm not sure I understand why these changes are necessary? We should strive to keep packages up-to-date with the module-template as much as possible. |
| "exactOptionalPropertyTypes": true, | ||
| "forceConsistentCasingInFileNames": true, | ||
| "lib": ["ES2020"], | ||
| "module": "CommonJS", | ||
| "moduleResolution": "node", | ||
| "noEmit": true, | ||
| "noErrorTruncation": true, | ||
| "noUncheckedIndexedAccess": true, |
There was a problem hiding this comment.
This seems like a downgrade to type safety.
There was a problem hiding this comment.
Hi, thanks for checking in on this! Since these flags are not enabled in the core monorepo's tsconfig, it was necessary to verify ahead of migration that disabling them won't break anything in this package.
My understanding is that exactOptionalPropertyTypes is in the process of being enabled in the core monorepo. Any further improvements in type safety will also propagate to this package once it's been migrated.
There was a problem hiding this comment.
Why can't this be done in the core repository itself? I don't think we should be merging this.
There was a problem hiding this comment.
This repo will be archived as soon as the eth-json-rpc-provider package is migrated, and all future development will take place in the core monorepo. #27 and #28 are preparatory steps for that process. These links might provide more context:
MetaMask/core#1707 (comment)
MetaMask/core#1551
MetaMask/core#1079
There was a problem hiding this comment.
- It's possible to have project-specific tsconfigs and overrides within a monorepo
- Perhaps we should consider upgrading typescript in core rather than downgrading it here? devDeps: @metamask/eslint-config*->12.1.0 core#1523
There was a problem hiding this comment.
-
@legobeat None of the non-root packages in the core monorepo seem to have project-specific compiler options. Is the monorepo set up to accommodate them without manual adjustments to compiler/linter/test settings? Also, is adding project-specific config in alignment with the intended direction of the monorepo? This is my first ticket, and I'm not entirely familiar with the build system or coding guidelines yet.
-
There are a number of deps where this package is ahead of the core repo. I don't know if it makes sense to wait for them to be updated upstream, when any version bumps in the core repo will be propagated to the
eth-json-rpc-providersubrepo once it's migrated.
There was a problem hiding this comment.
We can consider upgrading TypeScript in the monorepo rather than downgrading it here. Not sure why I hadn't thought of that, good suggestion.
Adding additional compiler options might be more difficult though. We can explore that too, but I recall trying to enable noUncheckedIndexedAccess in core once before and getting stuck on a type error caused by a library we use. That was a while ago though, we should try again.
But enabling these flags on a per-package basis might be a lot easier to get working, and provide a path to incrementally adding flags in the future. I haven't tried doing that.
|
Closing and consolidating into #28. New PR will avoid downgrading TypeScript version and removing compiler flags. |
…e monorepo - See MetaMask/core#1718 - See #27 - Closes MetaMask/core#1682
…e monorepo - See #1718 - See MetaMask/eth-json-rpc-provider#27 - Closes #1682
…of migration (#28) ## Description - This PR aligns the ESLint and Prettier packages and configs in this repo with the core monorepo. - This is a preparation step for migrating the `eth-json-rpc-provider` package into the core monorepo. - Originally, the plan was to decrement the TypeScript version to align with the core monorepo, but following [this discussion](#27), we've opted to use TypeScript version v4.8.4, and preserve the compiler options in this package that aren't enabled in the core monorepo. ## References - See MetaMask/core#1551 - Closes MetaMask/core#1683 - Closes MetaMask/core#1682 (is superseded by this PR)
…of migration (#28) ## Description - This PR aligns the ESLint and Prettier packages and configs in this repo with the core monorepo. - This is a preparation step for migrating the `eth-json-rpc-provider` package into the core monorepo. - Originally, the plan was to decrement the TypeScript version to align with the core monorepo, but following [this discussion](MetaMask/eth-json-rpc-provider#27), we've opted to use TypeScript version v4.8.4, and preserve the compiler options in this package that aren't enabled in the core monorepo. ## References - See #1551 - Closes #1683 - Closes #1682 (is superseded by this PR)
…e monorepo - See #1718 - See MetaMask/eth-json-rpc-provider#27 - Closes #1682
…of migration (#28) ## Description - This PR aligns the ESLint and Prettier packages and configs in this repo with the core monorepo. - This is a preparation step for migrating the `eth-json-rpc-provider` package into the core monorepo. - Originally, the plan was to decrement the TypeScript version to align with the core monorepo, but following [this discussion](MetaMask/eth-json-rpc-provider#27), we've opted to use TypeScript version v4.8.4, and preserve the compiler options in this package that aren't enabled in the core monorepo. ## References - See #1551 - Closes #1683 - Closes #1682 (is superseded by this PR)
eth-json-rpc-providerpackage into the core monorepo.~4.6.3core#1682eth-json-rpc-providerinto core monorepo core#1551