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

feat(composable): add a new composable #740

Merged
merged 2 commits into from
Jan 23, 2021

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.

None yet

2 participants