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.

Sign up for GitHub

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

Jump to bottom

remove all @ts-expect-error #4640

Closed
hardfist opened this issue Nov 14, 2023 · 9 comments · Fixed by #7864
Closed

remove all @ts-expect-error #4640

hardfist opened this issue Nov 14, 2023 · 9 comments · Fixed by #7864
Assignees
@SoonIter
Labels
refactor team The issue/pr is created by the member of Rspack.

Comments

@hardfist
Copy link
Contributor

#3863 introduces lots of //@ts-expect-error as workaround to enable typecheck test,
but we should fix and remove all of it
@hardfist hardfist changed the title remove all //@ts-expect-error introduced by https://github.com/web-infra-dev/rspack/pull/3863 remove all //@ts-expect-error Nov 14, 2023
@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Nov 14, 2023
@hardfist hardfist changed the title remove all //@ts-expect-error remove all @ts-expect-error Nov 14, 2023
@hardfist hardfist added the good first issue Good for newcomers label Nov 14, 2023
@HerringtonDarkholme
Copy link
Contributor

I would like to crush all the type errors in the code(call me TS error crusher). Would you like to assign this to me?

There are pretty many of them so would you think that curating a list first would be a better approach?

Thanks!

@hardfist
Copy link
Contributor Author

I would like to crush all the type errors in the code(call me TS error crusher). Would you like to assign this to me?

There are pretty many of them so would you think that curating a list first would be a better approach?

Thanks!

Yes, type gymnastics is back！

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Nov 15, 2023 •
edited
Loading

Let me first recap the current status of ts-error usage in the repo.

Overview

At the moment, more specifically eea9d06, rspack has 302 ts-expect-errors.

Package Name ts-expect-error Count
packages/rspack-cli 19
packages/rspack-dev-server 41
packages/rspack-plugin-html 18
packages/rspack-plugin-react-refresh 3
packages/rspack 211
scripts 1
webpack-test 9

Rspack core packages breakdown

Since rspack core packages itself has many ts-expect-error, a breakdown will be helpful to make every pull request change small and manageable.

Package Name ts-expect-error Count
src 53
src/builtin-loader 1
src/config 10
src/lib 29
src/loader-runner 25
src/logging 9
src/node 14
src/stats 18
src/util 50
test 2

Strategy

Type errors are nasty to see in codebase but should not impact runtime behavior.
To minimize the impact of fixing type error, changes will be split into small, standalone pull request and every change will aim at being type-level only.

Progress Tracking

  • packages/rspack-cli
  • packages/rspack-dev-server
  • packages/rspack-plugin-html
  • packages/rspack-plugin-react-refresh
  • scripts
  • webpack-test
  • packages/rspack
    • src
    • src/builtin-loader
    • src/config
    • src/lib
    • src/loader-runner
    • src/logging
    • src/node
    • src/stats
    • src/util
    • test

HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 15, 2023
@HerringtonDarkholme
fix(typing): fix ExternalObject type argument not found
a0fd1cc 
part of web-infra-dev#4640
@HerringtonDarkholme HerringtonDarkholme mentioned this issue Nov 15, 2023
Merged
2 tasks
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 15, 2023
@HerringtonDarkholme
fix(typing): fix ExternalObject type argument not found
bdfe82e 
part of web-infra-dev#4640
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 15, 2023
@HerringtonDarkholme
fix(typing): fix ExternalObject type argument not found
1d726f6 
part of web-infra-dev#4640
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 15, 2023
@HerringtonDarkholme
fix(typing): fix ExternalObject type argument not found
d541715 
part of web-infra-dev#4640
@hardfist
Copy link
Contributor Author

Please remember not modify any tsconfig during refactor, because it's easy to be broken like #4650

h-a-n-a pushed a commit to HerringtonDarkholme/rspack that referenced this issue Nov 15, 2023
@HerringtonDarkholme @h-a-n-a
fix(typing): fix ExternalObject type argument not found
1516e2e 
part of web-infra-dev#4640
github-merge-queue bot pushed a commit that referenced this issue Nov 15, 2023
@HerringtonDarkholme
fix(typing): fix ExternalObject type argument not found (#4651)
46dbbb1 
part of #4640
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 16, 2023
@HerringtonDarkholme
fix(typing): fix src/rspack-cli ts-expect-error
a6f3c6e 
part of web-infra-dev#4640
@HerringtonDarkholme HerringtonDarkholme mentioned this issue Nov 16, 2023
Merged
2 tasks
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 16, 2023
@HerringtonDarkholme
fix(typing): fix rspack-cli build typing error
45512b3 
part of web-infra-dev#4640
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Nov 16, 2023
@HerringtonDarkholme
fix(typing): fix rspack-cli utils type error
d50df17 
part of web-infra-dev#4640
hardfist pushed a commit that referenced this issue Nov 17, 2023
@HerringtonDarkholme
fix(typing): fix packages/rspack-cli ts-expect-error (#4673)
5764637 
* fix(typing): fix src/rspack-cli ts-expect-error

part of #4640

* fix(typing): fix rspack-cli build typing error

part of #4640

* fix(typing): fix rspack-cli utils type error

part of #4640
@xiaoxiangmoe xiaoxiangmoe mentioned this issue Nov 23, 2023
Merged
2 tasks
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Dec 2, 2023
@HerringtonDarkholme
fix: remove @ts-expect-error in rspack-plugin-html
fc38dfe 
part of web-infra-dev#4640
@HerringtonDarkholme HerringtonDarkholme mentioned this issue Dec 2, 2023
Merged
2 tasks
github-merge-queue bot pushed a commit that referenced this issue Dec 3, 2023
@HerringtonDarkholme
fix: remove @ts-expect-error in rspack-plugin-html (#4852)
10d4cb5 
part of #4640
@xiaoxiangmoe xiaoxiangmoe mentioned this issue Dec 5, 2023
Merged
2 tasks
HerringtonDarkholme added a commit to HerringtonDarkholme/rspack that referenced this issue Jan 7, 2024
@HerringtonDarkholme
fix: address expect-error in react-refresh and script
d9dd7b8 
part of web-infra-dev#4640
@HerringtonDarkholme HerringtonDarkholme mentioned this issue Jan 7, 2024
Merged
2 tasks
h-a-n-a pushed a commit that referenced this issue Jan 8, 2024
@HerringtonDarkholme
fix: address expect-error in react-refresh and script (#5253)
2d4ce52 
part of #4640
@stale Stale
Copy link

stale bot commented Mar 7, 2024

This issue has been automatically marked as stale because it has not had recent activity. If this issue is still affecting you, please leave any comment (for example, "bump"). We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the stale label Mar 7, 2024
@HerringtonDarkholme
Copy link
Contributor

bump

@stale stale bot removed the stale label Mar 7, 2024
@gaoachao gaoachao mentioned this issue Mar 10, 2024
Merged
2 tasks
@stale Stale
Copy link

stale bot commented May 6, 2024

This issue has been automatically marked as stale because it has not had recent activity. If this issue is still affecting you, please leave any comment (for example, "bump"). We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the stale label May 6, 2024
@stale stale bot removed the stale label Jul 9, 2024
@hardfist hardfist added stale refactor and removed stale good first issue Good for newcomers labels Jul 9, 2024
This was referenced Jul 12, 2024
Merged
Merged
Closed
@hardfist
Copy link
Contributor Author

hardfist commented Sep 5, 2024

@SoonIter is this finished?

@SoonIter
Copy link
Member

SoonIter commented Sep 6, 2024

@SoonIter is this finished?

not all but most of them, there are still several parts

  1. packages/rspack/loader-runner/index.js
  2. packages/rspack/src/stats/DefaultStatsFactoryPlugin.ts
  3. packages/rspack/src/MultiCompiler.ts
  4. packages/rspack/src/Watching.ts
  5. rspack-dev-server (cannot be removed because of private API)

and some one or two small pieces

This was referenced Sep 6, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SoonIter SoonIter

Labels
refactor team The issue/pr is created by the member of Rspack.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(types): remove @ts-expect-error in loaderRunner and StatsFactory SoonIter/rspack
3 participants
@HerringtonDarkholme @hardfist @SoonIter