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

Ts update #4869

Merged
merged 5 commits into from May 5, 2024
Merged

Ts update #4869

merged 5 commits into from May 5, 2024

Conversation

brunnre8
Copy link
Member

@brunnre8 brunnre8 commented May 4, 2024

Bump compiler to the latest, needs updated eslint as well, which lead to a bunch of errors that needed to be addressed.

@@ -93,6 +93,7 @@ const tsRules = defineConfig({
// note you must disable the base rule as it can report incorrect errors
"no-shadow": "off",
"@typescript-eslint/no-shadow": ["error"],
"@typescript-eslint/no-redundant-type-constituents": "off",
Copy link
Member Author

Choose a reason for hiding this comment

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

don't really see the point in making our types worse, even though it's technically correct that just having undefined as a type encompasses everything, but documentation wise that's poor.

expand: expand,
join: join,
search: search,
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This we probably also need to do with the other things that do the dynamic import...
While it does safe some typing, this is preventing us from moving to modules (we need top level imports or wait for the promise if we do it dynamically), is bad for tree shaking and simply put not needed as all our stuff is available at build time, we don't have plugins or such

Dynamic imports won't work very well with modules and we don't
really need them, it's just there to save us an import statement.

Let's flip this to a static version.
import {input as join} from "./join";
import {input as search} from "./search";

export const commands = {
Copy link
Member

@MaxLeiter MaxLeiter May 5, 2024

Choose a reason for hiding this comment

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

Nice, this is much simpler

@MaxLeiter MaxLeiter merged commit 74563ef into master May 5, 2024
12 checks passed
@MaxLeiter MaxLeiter deleted the tsUpdate branch May 5, 2024 04:49
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.

None yet

2 participants