Skip to content

Conversation

jy0529
Copy link
Contributor

@jy0529 jy0529 commented Jan 23, 2021

add a new compoasble useTimeout, #506

Copy link
Owner

@pikax pikax left a comment

Choose a reason for hiding this comment

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

LGTM

My only nit pick are the tests, because of awaiting for a timer to run off, there's a change it won't be executed property.

Thank you,

@pikax pikax merged commit e40ea53 into pikax:master Jan 23, 2021
@jy0529
Copy link
Contributor Author

jy0529 commented Jan 23, 2021

LGTM

My only nit pick are the tests, because of awaiting for a timer to run off, there's a change it won't be executed property.

Thank you,

wow, thanks, I will learn more about jest, and I refactor this tests, do you think this well?

`describe("timeout", () => {
beforeAll(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.clearAllTimers();
});

afterAll(() => {
jest.useRealTimers();
});

it("should be defined", () => {
expect(useTimeout).toBeDefined();
});

it("should call passed function after given amount of time", () => {
const callback = jest.fn();

useTimeout(callback, 1000);

expect(callback).not.toBeCalled();

jest.advanceTimersByTime(1000);
expect(callback).toBeCalled();
expect(callback).toHaveBeenCalledTimes(1);

});
......more tests
}
`

@jy0529
Copy link
Contributor Author

jy0529 commented Jan 23, 2021

you work so fast!

@pikax
Copy link
Owner

pikax commented Jan 23, 2021

wow, thanks, I will learn more about jest, and I refactor this tests, do you think this well?

Yep, I added it 0636f68 :)

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