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

build(deps): TypeScript 5.4 #3790

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

build(deps): TypeScript 5.4 #3790

wants to merge 4 commits into from

Conversation

erickzhao
Copy link
Member

@erickzhao erickzhao commented Dec 19, 2024

Our version of TypeScript is a few years out of date. DefinitelyTyped only supports versions that are 2 years old.

TypeScript support timeline

This PR upgrades us from TypeScript 4 to TypeScript 5.4, which is the latest version supported by our current version of TypeDoc.

The actual source code change here is fairly small, but upgrading to TypeScript 5 required us to upgrade our linting stack, which caused the diff to balloon:

  • Some linting rule names changed (e.g. @typescript-eslint/no-var-requires -> @typescript-eslint/no-require-imports).
  • The default ordering of eslint-plugin-import seems to have changed.

Note that we require a yarn resolution to @typescript-eslint/parser because @malept/eslint-config has a direct dependency on @typescript-eslint/parser@5, which limits us to TypeScript 4.6.

@erickzhao erickzhao marked this pull request as ready for review December 19, 2024 19:26
@erickzhao erickzhao requested a review from a team as a code owner December 19, 2024 19:26
@@ -294,7 +292,7 @@ export const listrMake = (
arch: targetArch,
});
} catch (err) {
if (err) {
Copy link
Member

Choose a reason for hiding this comment

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

This is semantically different, in some places we throw strings, we should still somehow log those?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are there specific places where we throw strings? This was caught in lint by "@typescript-eslint/only-throw-error".

Copy link
Member

Choose a reason for hiding this comment

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

I've definitely done it before, probably in forge, or it's dependencies 😅

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we still do it today or not

Copy link
Member Author

Choose a reason for hiding this comment

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

Added another else if clause for when the error is a string as well: 12efc76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants