Gh 9139 #2
Gh 9139 #2
Conversation
| } | ||
| } catch (e) { | ||
| console.error('Failed to refresh watch expressions: ', e); | ||
| protected refreshWatchExpressions: () => Promise<void> = debounceAsync(() => this.refreshWatchExpressionsQueue = this.refreshWatchExpressionsQueue.then(async () => { |
There was a problem hiding this comment.
The old code did not return the property assignment.
There was a problem hiding this comment.
I read the file,,,but not quite understand.
There was a problem hiding this comment.
The function you are debouncing is different from before, it shouldn't be.
There was a problem hiding this comment.
modified， pls check if it's correct now.
| @@ -106,7 +106,7 @@ export class ScmInput implements Disposable { | |||
| this.fireDidChange(); | |||
| } | |||
|
|
|||
| validate = debounce(async (): Promise<void> => { | |||
| validate: () => Promise<void> = debounceAsync(async (): Promise<void> => { | |||
There was a problem hiding this comment.
async () => { no debounce then
There was a problem hiding this comment.
No, with the debounce.
But I didn't see that the typing on arrow function wasn't from you. We can still remove it I assume?
There was a problem hiding this comment.
| validate: () => Promise<void> = debounceAsync(async (): Promise<void> => { | |
| validate: () => Promise<void> = debounceAsync(async () => { |
smart-bo
force-pushed
the
gh-9139
branch
5 times, most recently
from
2611401 to
882e023
Compare
| @@ -0,0 +1,28 @@ | |||
| /******************************************************************************** | |||
There was a problem hiding this comment.
Since debounceAsync is under promise-util.ts, the spec file should also be under that name promise-util.spec.ts.
| describe('debounceAsync', () => { | ||
|
|
||
| it('debounceAsync should return a promise', async () => { | ||
| const promise = await debounceAsync( | ||
| async () => 2 + 2, 50)(); | ||
| expect(promise).to.equal(4); | ||
| }); | ||
|
|
||
| }); |
There was a problem hiding this comment.
| describe('debounceAsync', () => { | |
| it('debounceAsync should return a promise', async () => { | |
| const promise = await debounceAsync( | |
| async () => 2 + 2, 50)(); | |
| expect(promise).to.equal(4); | |
| }); | |
| }); | |
| describe('promise-util', () => { | |
| describe('#debounceAsync', () => { | |
| it('should return a promise', async () => { | |
| const promise = await debounceAsync( | |
| async () => 2 + 2, 50)(); | |
| expect(promise).to.equal(4); | |
| }); | |
| }); | |
| }); |
|
|
||
| describe('debounceAsync', () => { | ||
|
|
||
| it('debounceAsync should return a promise', async () => { |
There was a problem hiding this comment.
Please use the form 'should... when describing individual tests.
| const promise = await debounceAsync( | ||
| async () => 2 + 2, 50)(); |
There was a problem hiding this comment.
The name promise is likely incorrect here, since it is awaited it is no longer a promise (as it has been resolved).
What it does
Fixes eclipse-theia#9139
We used more two libs for debouncing:
p-debounceandlodash.debounce,Since
lodash.debounceis used more thanp-debounce, we replacep-debouncebylodash.debouncewhere it used before.As
lodash.debouncereturnundefinewhere our API ask for a promise type, we have to write adebounceAsyncfunction to wraplodash.debounceand return an ideal type.This function is located at
@theia/core/lib/common/promise-util. Other files where is needed import it.How to test
Some of the functions which use debounce are difficult to test.
it would be better to test the
debounceAsyncfunction in a test file:packages/core/src/common/promise-util.spec.tsReview checklist
Reminder for reviewers