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

[quality] Use one lib for debounce. #9139

Open
kittaakos opened this issue Mar 1, 2021 · 7 comments · Fixed by smart-bo/theia#1 · May be fixed by smart-bo/theia#2
Open

[quality] Use one lib for debounce. #9139

kittaakos opened this issue Mar 1, 2021 · 7 comments · Fixed by smart-bo/theia#1 · May be fixed by smart-bo/theia#2
Labels
dependencies pull requests that update a dependency file help wanted issues meant to be picked up, require help quality issues related to code and application quality

Comments

@kittaakos
Copy link
Contributor

Bug Description:

Currently, we use more than one lib for debouncing:

Pick one and use that.

Steps to Reproduce:

Additional Information

  • Operating System:
  • Theia Version:
@vince-fugnitto vince-fugnitto added dependencies pull requests that update a dependency file help wanted issues meant to be picked up, require help quality issues related to code and application quality labels Mar 1, 2021
@vince-fugnitto vince-fugnitto added the good first issue good first issues for new contributors label Jul 15, 2021
@vince-fugnitto
Copy link
Member

I marked the issue as good first issue, if anyone would like to pick it up and needs some help please don't hesitate to ask :)

@123MayankSharma
Copy link

I marked the issue as good first issue, if anyone would like to pick it up and needs some help please don't hesitate to ask :)

Hello @vince-fugnitto ! I am new to open source world so would it be okay for me to take up this issue? Are there some prerequisites or guidelines that i should know before starting? It would be a great help if you can help me regarding this!

@vince-fugnitto
Copy link
Member

@123MayankSharma sure you can pick up the issue 👍

You can reference our readme for all information regarding building, and contributing, it should provide you with the information you need to contribute the change. Please feel free to ask any questions along the way as well:

@123MayankSharma
Copy link

@123MayankSharma sure you can pick up the issue

You can reference our readme for all information regarding building, and contributing, it should provide you with the information you need to contribute the change. Please feel free to ask any questions along the way as well:

Thanks a lot for the super quick reply! the help is really appreciated and hope to resolve this issue soon!

smart-bo added a commit to smart-bo/theia that referenced this issue Oct 29, 2021
smart-bo added a commit to smart-bo/theia that referenced this issue Oct 29, 2021
Replace `p-debounce` by `lodash.debounce`
smart-bo added a commit to smart-bo/theia that referenced this issue Oct 29, 2021
smart-bo added a commit to smart-bo/theia that referenced this issue Nov 1, 2021
@smart-bo smart-bo linked a pull request Nov 2, 2021 that will close this issue
1 task
@JonasHelming
Copy link
Contributor

@vince-fugnitto : I believe this is actually fixed, do you agree?

@JonasHelming JonasHelming removed the good first issue good first issues for new contributors label May 7, 2022
@vince-fugnitto
Copy link
Member

@vince-fugnitto : I believe this is actually fixed, do you agree?

@JonasHelming can you clarify why believe it is currently fixed, afaik we still make use of both debounce libraries:

p-debounce:

 $ yarn why p-debounce
yarn why v1.22.4
[1/4] Why do we have the module "p-debounce"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#@theia#core" depends on it
   - Hoisted from "_project_#@theia#core#p-debounce"
   - Hoisted from "_project_#@theia#debug#p-debounce"
   - Hoisted from "_project_#@theia#preferences#p-debounce"
   - Hoisted from "_project_#@theia#scm#p-debounce"
   - Hoisted from "_project_#@theia#task#p-debounce"
   - Hoisted from "_project_#@theia#vsx-registry#p-debounce"
info Disk size without dependencies: "20KB"
info Disk size with unique dependencies: "20KB"
info Disk size with transitive dependencies: "20KB"
info Number of shared dependencies: 0
Done in 1.75s.

lodash.debounce:

$ yarn why lodash.debounce
yarn why v1.22.4
[1/4] Why do we have the module "lodash.debounce"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#@theia#core" depends on it
   - Hoisted from "_project_#@theia#core#lodash.debounce"
   - Hoisted from "_project_#@theia#application-manager#@babel#plugin-transform-runtime#babel-plugin-polyfill-corejs2#@babel#helper-define-polyfill-provider#lodash.debounce"
info Disk size without dependencies: "24KB"
info Disk size with unique dependencies: "24KB"
info Disk size with transitive dependencies: "24KB"
info Number of shared dependencies: 0
Done in 1.12s.

I however believe that the issue is likely not as critical as first thought, both libraries do things differently and handle different use-cases (async vs sync for example).

@JonasHelming
Copy link
Contributor

I was just looking at smart-bo#1, but I misunderstood that apparently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file help wanted issues meant to be picked up, require help quality issues related to code and application quality
Projects
None yet
4 participants