Skip to content

fix(packages): Remove unnecessary code and add types for untyped code and prettify everything#2022

Closed
PatrykKuniczak wants to merge 64 commits intowxt-dev:mainfrom
PatrykKuniczak:fix/remove-unnecessary-code-and-add-missing-types
Closed

fix(packages): Remove unnecessary code and add types for untyped code and prettify everything#2022
PatrykKuniczak wants to merge 64 commits intowxt-dev:mainfrom
PatrykKuniczak:fix/remove-unnecessary-code-and-add-missing-types

Conversation

@PatrykKuniczak
Copy link
Collaborator

@PatrykKuniczak PatrykKuniczak commented Dec 25, 2025

Overview

  1. Cleaned everything a little
  2. Make more types, for avoid any and @ts-expect-error and @ts-ignore
  3. Fixed some typos
  4. Replaced deprecated methods/props to newer options
  5. Make string literals and numbers UPPER_CASE like real const
  6. Remove unnecessary await, before return
  7. Remove unused methods

I'm also added some questions to maintainers under TODO: keys (let's response, please 😄)

PS: I can make it a little better, by configure more restrictions for eslint and prettier(e.g brackets near to ifs, func and etc.) in other PR and reduce amount of tests, because IMO some of those isn't necessary(e.g this which TS preventing by itself) and we can remove it.

Tell me if you overall, interested for more my influence on this project 👀

Manual Testing

Run tests and pnpm test and pnpm dev and check if everything's alright :)

@netlify
Copy link

netlify bot commented Dec 25, 2025

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit 5c7c647
🔍 Latest deploy log https://app.netlify.com/projects/creative-fairy-df92c4/deploys/69885684597641000864005e
😎 Deploy Preview https://deploy-preview-2022--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@PatrykKuniczak PatrykKuniczak force-pushed the fix/remove-unnecessary-code-and-add-missing-types branch 3 times, most recently from f1070ec to ff88495 Compare December 29, 2025 20:11
@PatrykKuniczak PatrykKuniczak marked this pull request as ready for review December 29, 2025 20:12
@PatrykKuniczak PatrykKuniczak changed the title Fix/remove unnecessary code and add missing types fix(packages): Remove unnecessary code and add types for untyped code and prettify everything Dec 29, 2025
@aklinker1
Copy link
Member

PRs need to be small and targetted, I can't review a PR this large. Can you break it up into multiple smaller ones? For example, one for removing @ts-ignore comments, one for fixing typos, one for cleaning up unused code, etc.

That way changes I don't have feedback on can be merged quickly without being blocked by comments on the other changes.

PS: I can make it a little better, by configure more restrictions for eslint and prettier(e.g brackets near to ifs, func and etc.) in other PR and reduce amount of tests, because IMO some of those isn't necessary(e.g this which TS preventing by itself) and we can remove it.

Linting and prettier are already setup exactly how I want them - with the defaults. I fall in the camp of just rolling with the defaults when available, less to worry about. So no, I don't want to make additional changes to their config.

If PR checks pass, that's fine by me. We don't need super strict rules for everything.

@PatrykKuniczak
Copy link
Collaborator Author

PRs need to be small and targetted, I can't review a PR this large. Can you break it up into multiple smaller ones? For example, one for removing @ts-ignore comments, one for fixing typos, one for cleaning up unused code, etc.

Ok, i'll do it someday

@PatrykKuniczak
Copy link
Collaborator Author

@aklinker1 What's the reason, you don't want to make configs more strict?

@aklinker1
Copy link
Member

What's the reason, you don't want to make configs more strict?

This isn't an enterprise project, we don't need to enforce every single nit and opinion. As an open source project, I'd rather not worry about all that and just go with the flow - oxlint's defaults already do a good job of preventing bugs and keeping code style consistent enough, everything past that is just opinions I don't care about enforcing and that do not provide a meaningful improvement to the project.

Just because one person likes having curly braces after every if/else statement doesn't mean it's worth my time to review a PR going through the entire codebase and making a big PR to change them or modifying the git blame.

The 15 minutes I've spent writing this up is already more time than I'd like to spend on these sorts of things.

The only linting rules I would be willing to add are custom ones that enforce custom requirements specific to this repo - but none of those exist right now and I'd prefer to keep the repo simple enough that those are never needed.

@aklinker1
Copy link
Member

TLDR; I believe the defaults already provide the consistency this project needs.

…CASE like real const and remove unnecessary `exists` func and use `pathExists` instead
…ssary `await`, make closeBrowser function of ExtensionRunner optional
@PatrykKuniczak PatrykKuniczak force-pushed the fix/remove-unnecessary-code-and-add-missing-types branch from ff88495 to 5c7c647 Compare February 8, 2026 09:25
@PatrykKuniczak
Copy link
Collaborator Author

This PR will be split.
First part: #2081
Second part will be opened after first will be merged(Will contain @ts-expect-errors and types fixes)

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