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

chore(types): add test for rendering SVG elements #834

Merged
merged 1 commit into from
Nov 19, 2020

Conversation

nickserv
Copy link
Member

What: Adding regression tests for #830 (implemented in #833)

Why: It helps to have a regression test for the original issue when adding a fix

How: TypeScript type test based on sample code in #830

Checklist:

  • Documentation added to the
    docs site N/A
  • Tests
  • Typescript definitions updated N/A
  • Ready to be merged

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 307bd41:

Sandbox Source
React Configuration
react-testing-library-examples Configuration

@codecov
Copy link

codecov bot commented Nov 19, 2020

Codecov Report

Merging #834 (307bd41) into master (1dc33b2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #834   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          140       140           
  Branches        28        28           
=========================================
  Hits           140       140           

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 1dc33b2...307bd41. Read the comment docs.

@nickserv
Copy link
Member Author

I wonder if there's a way to get TypeScript test coverage using a logical specification DSL

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks!

@kentcdodds kentcdodds merged commit 276673f into master Nov 19, 2020
@kentcdodds kentcdodds deleted the test-include-svg-element-types branch November 19, 2020 14:45
@github-actions
Copy link

🎉 This PR is included in version 11.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@just-boris
Copy link
Contributor

Why was this released as a minor version bump?

This breaks the following code:

function findSomething(element: HTMLElement) {...}


const {container} = render(...);

findSomething(container);
//            ~~~~~~~~~
// Argument of type 'Element' is not assignable to parameter of type 'HTMLElement'.
//  Type 'Element' is missing the following properties from type 'HTMLElement': accessKey, accessKeyLabel, autocapitalize, // dir, and 107 more.(2345)

This is a breaking change, and should have waited until the major version 12

@eps1lon
Copy link
Member

eps1lon commented Jan 26, 2021

This is a breaking change, and should have waited until the major version

Sorry to hear you're having trouble with the latest patch.

You probably mean #833. The PR you commented on did not affect published code.

Note that any change is potentially backwards incompatible. Especially type related changes are very likely to break some code. This change in particular was bug fix. If your code relied on a bug then the change is backwards incompatible for you. Note that any @types/* package is already pushing type changes in patch versions for this exact reason.

However, we cannot always make allowances in these instances. Otherwise we would need to release any change with a major version.

@just-boris
Copy link
Contributor

Oh correct, I mixed these two PRs up.

I assumed that the signature of the public API remains unchanged. E.g. if render() function returns a certain type, it will continue returning it until the next major release.

Do you have any suggestions on how to protect against unexpected changes? The pattern is very common, there is a real example (last line in the file) which made me respond to this PR.

@eps1lon
Copy link
Member

eps1lon commented Jan 26, 2021

Do you have any suggestions on how to protect against unexpected changes?

Working on a fix right now.

@eps1lon
Copy link
Member

eps1lon commented Jan 26, 2021

@just-boris Subscribe to #868 for further updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants