Skip to content

Commit

Permalink
chore: update typescript to ~v5.3.3 (#22530)
Browse files Browse the repository at this point in the history
This PR updates our typescript version to 5.3.3 (currently the latest
version).

It also updates some eslint and babel dependencies as the older
versions didn't work with the newer version of typescript.

There were some rule changes:

1. `jsdoc/check-line-alignment` now requires consistent line wrap
indentation. This is a large change that I don't want to make in this PR
(it should be separate and the commit added to a
`.git-blame-ignore-revs` file).
2. `Object<...>` type syntax in jsdoc is not allowed; I fixes the
breakages in this PR, as they were minimal
3. JS files that were checked using `typescript-eslint` aren't anymore.
It is no longer permitted to apply `typescript-eslint` to files that are
not also included in our `tsconfig.json`, and since our JS does not pass
the typechecker I had to remove `typescript-eslint` from some JS rules.
4. We previously had a rule `jest/no-large-snapshots`, but it wasn't
actually enforced. This eslint package updates fix the rule, but we
don't even come close to passing it, so I turned it off
5. ui/store/actions.ts was beyond repair. I've enabled `@ts-nocheck` for
the entire file. I do NOT volunteer as tribute to fix this. :-)

I have also simplified and updated our `tsconfig.json` to remove
redundant options and align it with the features and files that we use.
I've also added compilation caching, which makes successive typescript
linting much faster. See comments in `tsconfig.json`.

I've merged our `.prettierignore` and `eslint` ignore lists together, as
the two lists seem to have diverged over time. Our `eslintrc.js` now
imports `.prettierignore` so they won't diverge again in the future.

Our eslint config now fully parses our `tsconfig.json` and only applies
`typescript-eslint` to `.ts` and `.tsx` files that our tsconfig.json
lists (the `.js` files don't typecheck sucessfully, so they aren't
included).
  • Loading branch information
davidmurdoch authored Feb 15, 2024
1 parent 80d0822 commit 8e00507
Show file tree
Hide file tree
Showing 17 changed files with 384 additions and 394 deletions.
2 changes: 1 addition & 1 deletion .depcheckrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ ignores:
- '@metamask/phishing-warning' # statically hosted as part of some e2e tests
- '@metamask/test-dapp'
- '@metamask/design-tokens' # Only imported in index.css
- '@tsconfig/node16' # required dynamically by TS, used in tsconfig.json
- '@tsconfig/node20' # required dynamically by TS, used in tsconfig.json
- '@sentry/cli' # invoked as `sentry-cli`
- 'chromedriver'
- 'depcheck' # ooo meta
Expand Down
3 changes: 3 additions & 0 deletions .eslintrc.base.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ module.exports = {
},

rules: {
// TODO: re-enable once the proposed feature at https://github.com/gajus/eslint-plugin-jsdoc/pull/964#issuecomment-1936470252 is available
'jsdoc/check-line-alignment': 'off',

'default-param-last': 'off',
'prefer-object-spread': 'error',
'require-atomic-updates': 'off',
Expand Down
73 changes: 47 additions & 26 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
const path = require('path');

const { readFileSync } = require('node:fs');
const path = require('node:path');
const ts = require('typescript');
const { version: reactVersion } = require('react/package.json');

const tsconfigPath = ts.findConfigFile('./', ts.sys.fileExists);
const { config } = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
const tsconfig = ts.parseJsonConfigFileContent(config, ts.sys, './');

/**
* @type {import('eslint').Linter.Config }
*/
module.exports = {
root: true,
// Suggested addition from the storybook 6.5 update
extends: ['plugin:storybook/recommended'],
// Ignore files which are also in .prettierignore
ignorePatterns: [
'app/vendor/**',
'builds/**/*',
'development/chromereload.js',
'development/charts/**',
'development/ts-migration-dashboard/build/**',
'dist/**/*',
'node_modules/**/*',
'storybook-build/**/*',
'jest-coverage/**/*',
'coverage/**/*',
'public/**/*',
],
ignorePatterns: readFileSync('.prettierignore', 'utf8').trim().split('\n'),
// eslint's parser, esprima, is not compatible with ESM, so use the babel parser instead
parser: '@babel/eslint-parser',
overrides: [
/**
* == Modules ==
Expand Down Expand Up @@ -125,13 +123,39 @@ module.exports = {
* TypeScript files
*/
{
files: ['*.{ts,tsx}'],
files: tsconfig.fileNames.filter((f) => /\.tsx?$/u.test(f)),
parserOptions: {
project: tsconfigPath,
// https://github.com/typescript-eslint/typescript-eslint/issues/251#issuecomment-463943250
tsconfigRootDir: path.dirname(tsconfigPath),
},
extends: [
path.resolve(__dirname, '.eslintrc.base.js'),
'@metamask/eslint-config-typescript',
path.resolve(__dirname, '.eslintrc.typescript-compat.js'),
],
rules: {
// this rule is new, but we didn't use it before, so it's off now
'@typescript-eslint/no-duplicate-enum-values': 'off',
'@typescript-eslint/no-shadow': [
'error',
{
builtinGlobals: true,
allow: [
'ErrorOptions',
'Text',
'Screen',
'KeyboardEvent',
'Lock',
'Notification',
'CSS',
],
},
],
// `no-parameter-properties` was removed in favor of `parameter-properties`
// Yeah, they have opposite names but do the same thing?!
'@typescript-eslint/no-parameter-properties': 'off',
'@typescript-eslint/parameter-properties': 'error',
// Turn these off, as it's recommended by typescript-eslint.
// See: <https://typescript-eslint.io/docs/linting/troubleshooting#eslint-plugin-import>
'import/named': 'off',
Expand Down Expand Up @@ -298,13 +322,12 @@ module.exports = {
rules: {
'import/unambiguous': 'off',
'import/named': 'off',
'jest/no-large-snapshots': [
'error',
{
maxSize: 50,
inlineMaxSize: 50,
},
],
// *.snap files weren't parsed by previous versions of this eslint
// config section, but something got fixed somewhere, and now this rule
// causes failures. We need to turn it off instead of fix them because
// we aren't even remotely close to being in alignment. If it bothers
// you open a PR to fix it yourself.
'jest/no-large-snapshots': 'off',
'jest/no-restricted-matchers': 'off',

/**
Expand Down Expand Up @@ -398,13 +421,11 @@ module.exports = {
},
},
{
files: ['ui/components/multichain/**/*.{js,ts,tsx}'],
files: ['ui/components/multichain/**/*.{js}'],
extends: [
path.resolve(__dirname, '.eslintrc.base.js'),
path.resolve(__dirname, '.eslintrc.node.js'),
path.resolve(__dirname, '.eslintrc.babel.js'),
path.resolve(__dirname, '.eslintrc.typescript-compat.js'),
'@metamask/eslint-config-typescript',
],
rules: {
'sort-imports': [
Expand Down
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ notes.txt
.metamaskrc
.metamaskprodrc

# TypeScript
tsout/

# Test results
test-results/

Expand Down
23 changes: 13 additions & 10 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
node_modules/**
node_modules/**/*
lavamoat/**/policy.json
dist/**
builds/**
test-*/**
coverage/
jest-coverage/
storybook-build/
dist/**/*
builds/**/*
test-*/**/*
app/vendor/**
.nyc_output/**
test/e2e/send-eth-with-private-key-test/**
.nyc_output/**/*
test/e2e/send-eth-with-private-key-test/**/*
*.scss
development/chromereload.js
development/ts-migration-dashboard/filesToConvert.json
public
development/ts-migration-dashboard/build/**
public/**/*
development/charts/**
node_modules/**/*
storybook-build/**/*
jest-coverage/**/*
coverage/**/*
1 change: 1 addition & 0 deletions app/scripts/lib/SnapsNameProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
AddressLookupResult,
Snap as TruncatedSnap,
} from '@metamask/snaps-sdk';
// @ts-expect-error see: https://github.com/MetaMask/snaps/pull/2174
import { HandlerType } from '@metamask/snaps-utils';
import log from 'loglevel';
import {
Expand Down
1 change: 1 addition & 0 deletions app/scripts/lib/snap-keyring/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { RestrictedControllerMessenger } from '@metamask/base-controller';
import { KeyringControllerGetKeyringForAccountAction } from '@metamask/keyring-controller';
import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
import { GetSnap } from '@metamask/snaps-controllers';
// @ts-expect-error see: https://github.com/MetaMask/snaps/pull/2174
import { Snap } from '@metamask/snaps-utils';

type AllowedActions =
Expand Down
56 changes: 32 additions & 24 deletions development/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,38 @@ const createEtcTasks = require('./etc');
const { getBrowserVersionMap, getEnvironment } = require('./utils');
const { getConfig } = require('./config');

// Packages required dynamically via browserify configuration in dependencies
// Required for LavaMoat policy generation
require('loose-envify');
require('@babel/preset-env');
require('@babel/preset-react');
require('@babel/preset-typescript');
require('@babel/core');
// ESLint-related
require('@babel/eslint-parser');
require('@babel/eslint-plugin');
require('@metamask/eslint-config');
require('@metamask/eslint-config-nodejs');
require('@typescript-eslint/parser');
require('eslint');
require('eslint-config-prettier');
require('eslint-import-resolver-node');
require('eslint-import-resolver-typescript');
require('eslint-plugin-import');
require('eslint-plugin-jsdoc');
require('eslint-plugin-node');
require('eslint-plugin-prettier');
require('eslint-plugin-react');
require('eslint-plugin-react-hooks');
require('eslint-plugin-jest');
/* eslint-disable no-constant-condition, node/global-require */
if (false) {
// Packages required dynamically via browserify/eslint configuration in
// dependencies. This is a workaround for LavaMoat's static analyzer used in
// policy generation. To avoid the case where we need to write policy
// overrides for these packages we can plop them here and they will be
// included in the policy. Neat!
require('loose-envify');
require('@babel/preset-env');
require('@babel/preset-react');
require('@babel/preset-typescript');
require('@babel/core');
// ESLint-related
require('@babel/eslint-parser');
require('@babel/eslint-plugin');
require('@metamask/eslint-config');
require('@metamask/eslint-config-nodejs');
// eslint-disable-next-line import/no-unresolved
require('@typescript-eslint/parser');
require('eslint');
require('eslint-config-prettier');
require('eslint-import-resolver-node');
require('eslint-import-resolver-typescript');
require('eslint-plugin-import');
require('eslint-plugin-jsdoc');
require('eslint-plugin-node');
require('eslint-plugin-prettier');
require('eslint-plugin-react');
require('eslint-plugin-react-hooks');
require('eslint-plugin-jest');
}
/* eslint-enable no-constant-condition, node/global-require */

defineAndRunBuildTasks().catch((error) => {
console.error(error.stack || error);
Expand Down
Loading

0 comments on commit 8e00507

Please sign in to comment.