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: Update all dependencies #190

Merged
merged 3 commits into from
Jan 24, 2020
Merged

chore: Update all dependencies #190

merged 3 commits into from
Jan 24, 2020

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Jan 22, 2020

No description provided.

package.json Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 22, 2020

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #190   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          19     19           
  Lines         235    235           
  Branches       57     57           
=====================================
  Hits          235    235
Impacted Files Coverage Δ
src/to-be-invalid.js 100% <ø> (ø) ⬆️

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 e4d61c2...66a92a0. Read the comment docs.

package.json Outdated
},
"eslintConfig": {
"extends": "./node_modules/kcd-scripts/eslint.js",
"rules": {
"babel/no-invalid-this": "off",
"import/prefer-default-export": "off",
"import/no-unassigned-import": "off"
"import/no-extraneous-dependencies": "off",
Copy link
Member

Choose a reason for hiding this comment

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

Why is this rule complaining all of the sudden?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's because of an update in kcd-scripts' ESLint config

Copy link
Member

Choose a reason for hiding this comment

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

So it wasn't there before? What is it complaining about?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's complaining about jsdom not in our dependencies.

Copy link
Member Author

@MichaelDeBoey MichaelDeBoey Jan 22, 2020

Choose a reason for hiding this comment

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

Maybe we should just ignore import/no-extraneous-dependencies in __tests__? 🤔

Copy link
Collaborator

@jgoz jgoz Jan 22, 2020

Choose a reason for hiding this comment

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

jsdom used to be in devDependencies but it got removed in this PR. Rather than disabling the rule, could the dependency be restored?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jgoz It's included by jest-environment-jsdom-sixteen

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, but there is a require("jsdom") statement inside this repository, which technically makes it an explicit (dev) dependency of this project. Disabling the rule as a whole will prevent us from detecting when any transitive dependency is imported as though it were a direct dependency, not just this one instance.

@weyert
Copy link

weyert commented Jan 22, 2020

Shouldn’t we also increase the minimum version of node in engines from 8 to 10?

@MichaelDeBoey
Copy link
Member Author

@weyert That's what this discussion was about 🙂
#190 (comment)

@gnapse gnapse requested a review from eps1lon January 22, 2020 16:58
@weyert
Copy link

weyert commented Jan 22, 2020 via email

Copy link
Collaborator

@jgoz jgoz left a comment

Choose a reason for hiding this comment

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

Couple requests from me - thanks for doing this.

One other thing: it looks like extend-expect.d.ts is still listed in files in package.json. Could you please remove that entry while you're here? I forgot to remove it in a previous PR.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@MichaelDeBoey
Copy link
Member Author

@jgoz I've updated as you requested 🙂

@MichaelDeBoey MichaelDeBoey requested a review from jgoz January 23, 2020 09:19
Copy link

@weyert weyert left a comment

Choose a reason for hiding this comment

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

Looking good to me :) I

Copy link
Collaborator

@jgoz jgoz left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you

@gnapse
Copy link
Member

gnapse commented Jan 24, 2020

This is supposed to be a patch release, right? Or is it a minor release?

@kentcdodds
Copy link
Member

🤷‍♂️ I think I'd call this a patch, but I don't have strong feelings either way.

@gnapse gnapse merged commit c9a8664 into testing-library:master Jan 24, 2020
@gnapse
Copy link
Member

gnapse commented Jan 24, 2020

@all-contributors please add @MichaelDeBoey for infra

@allcontributors
Copy link
Contributor

@gnapse

I've put up a pull request to add @MichaelDeBoey! 🎉

@MichaelDeBoey MichaelDeBoey deleted the update-dependencies branch January 24, 2020 06:10
@gnapse
Copy link
Member

gnapse commented Jan 31, 2020

🎉 This PR is included in version 5.1.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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants