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

Dom Ready: Add types #19089

Merged
merged 5 commits into from Dec 12, 2019
Merged

Dom Ready: Add types #19089

merged 5 commits into from Dec 12, 2019

Conversation

@sirreal
Copy link
Member

sirreal commented Dec 12, 2019

Description

Part of #18838
Extracted from #18942

Add types to dom-ready package.

Testing Instructions:

Verify type checking passes:

npm run lint-types
@aduth aduth mentioned this pull request Dec 12, 2019
9 of 67 tasks complete
packages/dom-ready/README.md Outdated Show resolved Hide resolved
if (
document.readyState === 'complete' || // DOMContentLoaded + Images/Styles/etc loaded, so we call directly.
document.readyState === 'interactive' // DOMContentLoaded fires at this point, so we call directly.
) {
return callback();
return void callback();

This comment has been minimized.

Copy link
@sirreal

sirreal Dec 12, 2019

Author Member

Subtle behavioral change - this function did not always return void as its type suggested. In this branch, tt would return whatever the callback returned.

Adding the void here so the function always returns undefined.

This comment has been minimized.

Copy link
@aduth

aduth Dec 12, 2019

Member

Subtle behavioral change - this function did not always return void as its type suggested. In this branch, tt would return whatever the callback returned.

Adding the void here so the function always returns undefined.

I think this syntax can be a little unexpected for some people, or at least it was for me the first times I had seen it, but I think it's a nice simple way to guarantee the return type as undefined, as you suggest.

@sirreal sirreal marked this pull request as ready for review Dec 12, 2019
@sirreal sirreal requested a review from aduth as a code owner Dec 12, 2019
Copy link
Member

aduth left a comment

This will need a rebase after #19099 was merged (it will take some time for the "include" to populate a bit more before we can avoid such frequent merge conflicts 😬 ).

@sirreal sirreal force-pushed the add/types-dom-ready branch from 47bcfaf to 8489ad3 Dec 12, 2019
@sirreal

This comment has been minimized.

Copy link
Member Author

sirreal commented Dec 12, 2019

Rebased.

@sirreal sirreal force-pushed the add/types-dom-ready branch from 58117e8 to ac78666 Dec 12, 2019
@sirreal sirreal requested a review from aduth Dec 12, 2019
@aduth
aduth approved these changes Dec 12, 2019
@sirreal sirreal merged commit 674a0c9 into master Dec 12, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@sirreal sirreal deleted the add/types-dom-ready branch Dec 12, 2019
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.