Skip to content

fix: update config type#598

Merged
kentcdodds merged 1 commit into
masterfrom
pr/config-type
Jun 1, 2020
Merged

fix: update config type#598
kentcdodds merged 1 commit into
masterfrom
pr/config-type

Conversation

@timdeschryver

@timdeschryver timdeschryver commented May 31, 2020

Copy link
Copy Markdown
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

codesandbox-ci Bot commented May 31, 2020

Copy link
Copy Markdown

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

codecov Bot commented May 31, 2020 •
edited
Loading

Copy link
Copy Markdown

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

kentcdodds commented May 31, 2020

Copy link
Copy Markdown
Member

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

@benmonro

benmonro commented Jun 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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.

kentcdodds
kentcdodds approved these changes Jun 1, 2020
View reviewed changes

@kentcdodds kentcdodds left a comment

Copy link
Copy Markdown
Member

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

kentcdodds commented Jun 1, 2020

Copy link
Copy Markdown
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

Reviewers

@kentcdodds kentcdodds kentcdodds approved these changes

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@timdeschryver @kentcdodds @benmonro