Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Feature/beta improvements #532

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

Merged
mpeyper merged 6 commits into beta from feature/beta-improvements
Jan 11, 2021
Merged

Feature/beta improvements #532

mpeyper merged 6 commits into beta from feature/beta-improvements
Jan 11, 2021

Conversation

Copy link
Member

@mpeyper mpeyper commented Jan 10, 2021

What:

Fixed up some things:

  1. Cleanup up some types
  2. The type exports should now by compatible the the pre-v4 DefinitelyTyped types (discord comment)
  3. A potential bug that has existed since v3.6 when result.all was introduced

Checklist:

  • Tests
  • Ready to be merged

Copy link

codecov bot commented Jan 10, 2021
edited
Loading

Codecov Report

Merging #532 (54f116b) into beta (43891e1) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@
## beta #532 +/- ##
=========================================
 Coverage 100.00% 100.00% 
=========================================
 Files 12 14 +2 
 Lines 189 203 +14 
 Branches 27 29 +2 
=========================================
+ Hits 189 203 +14 
Impacted Files Coverage Δ
src/dom/pure.ts 100.00% <ø> (ø)
src/helpers/createTestHarness.tsx 100.00% <ø> (ø)
src/native/pure.ts 100.00% <ø> (ø)
src/pure.ts 100.00% <ø> (ø)
src/server/pure.ts 100.00% <ø> (ø)
src/core/cleanup.ts 100.00% <100.00%> (ø)
src/core/index.ts 100.00% <100.00%> (ø)
src/index.ts 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43891e1...54f116b. Read the comment docs.

Copy link
Member Author

mpeyper commented Jan 11, 2021

I'm not sure why the build has suddenly started caring about the lack of tests on src/index.ts and src/pure.ts... Probably the kcd-scripts changes?

Guess I'm looking at how we go about testing that next then.

},
get current() {
const { value, error } = results[results.length - 1]
const { value, error } = results[results.length - 1] ?? {}
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the bug

Comment on lines +21 to +22
const expectedRenderer = require('../dom/pure') as ReactHooksRenderer
const actualRenderer = require('..') as ReactHooksRenderer
Copy link
Member Author

Choose a reason for hiding this comment

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

had to use require in these tests otherwise the imports have resolved before the mocking occurs

Copy link
Member

@joshuaellis joshuaellis Jan 11, 2021

Choose a reason for hiding this comment

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

This is really interesting, I wasn't sure how to do this myself.

Copy link
Member

@joshuaellis joshuaellis left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mpeyper mpeyper merged commit dc21e59 into beta Jan 11, 2021
Copy link

🎉 This PR is included in version 5.0.0-beta.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mpeyper mpeyper deleted the feature/beta-improvements branch January 11, 2021 11:46
Copy link

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshuaellis joshuaellis joshuaellis approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /