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

fix: update config type #598

Merged
merged 1 commit into from Jun 1, 2020
Merged

fix: update config type #598

merged 1 commit into from Jun 1, 2020

Conversation

timdeschryver
Copy link
Member

What: We forgot to add the throwSuggestions option in the global config

Why: To have intellisense. In TS projects we also currently get an error while trying to do the following.

configure({ throwSuggestions: true });

How:

Checklist:

Should we try to keep the naming consistent?
For the global config it's throwSuggestions , while the config on a query basis is suggest.

configure({throwSuggestions: true})
// vs
screen.getByText('Increment', {suggest: true})

@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 c76c1b5:

Sandbox Source
compassionate-babbage-ew0uf Configuration

@codecov
Copy link

codecov bot commented May 31, 2020

Codecov Report

Merging #598 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #598   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          536       536           
  Branches       133       133           
=========================================
  Hits           536       536           

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 bec190b...c76c1b5. Read the comment docs.

@kentcdodds
Copy link
Member

Yes, these should be consistent. That was an oversight. I suggest we add the option and deprecate the old one.

@benmonro
Copy link
Member

benmonro commented Jun 1, 2020

Should we try to keep the naming consistent?
For the global config it's throwSuggestions , while the config on a query basis is suggest.

@timdeschryver @kentcdodds actually I think it's ok to keep the naming different between the config & the inline option. Actually I'm hoping to add a way to choose between doing a warn and a throw, but in either case you'd still want to turn it off for a specific query so I think it's fine to keep them slightly different.

also thanks for putting this PR in, thought i got all the typings, but must have missed it.

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.

Let's get this merged now and we can talk about naming conventions later.

@kentcdodds kentcdodds merged commit eeb11ec into master Jun 1, 2020
@kentcdodds kentcdodds deleted the pr/config-type branch June 1, 2020 01:53
@kentcdodds
Copy link
Member

🎉 This PR is included in version 7.7.3 🎉

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants