Movatterモバイル変換


[0]ホーム

URL:


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

BREAKING CHANGE: Update deps#777

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
joshuaellis wants to merge2 commits intomain
base:main
Choose a base branch
Loading
fromchore/update-deps
Draft

Conversation

joshuaellis
Copy link
Member

@joshuaellisjoshuaellis commentedJan 18, 2022
edited
Loading

What:

  • BREAKING CHANGE: min supported version of node is 12.22
  • Updates the deps we have in the package
  • Updates the workflow for testing against node versions

Why:

How:

Checklist:

  • Documentation updated
  • Tests
  • Ready to be merged
  • Added myself to contributors table

BREAKING CHANGE: min supported version of node is 12.22
@joshuaellis
Copy link
MemberAuthor

I don't expect this to pass on first go, but wanted to get the ball rolling@mpeyper

Also on the subject of#729, my POV is move them to devDeps and not include them as peer deps, this is what other libraries typically do from my experience. Happy to be told otherwise though.

@netlify
Copy link

netlifybot commentedJan 18, 2022
edited
Loading

❌ Deploy Preview forreact-hooks-testing-library failed.

🔨 Explore the source changes:416d475

🔍 Inspect the deploy log:https://app.netlify.com/sites/react-hooks-testing-library/deploys/61efcdc260b71200088417d4

@joshuaellis
Copy link
MemberAuthor

This is a strange error to have...

@mpeyper
Copy link
Member

The rules not being found?

I believedocz (or one of the related packages) is bringing in an incorrect version ofeslint which is why we had the version fromkcd-scripts in ourpackage.json to begin with.

@joshuaellis
Copy link
MemberAuthor

I believe docz (or one of the related packages) is bringing in an incorrect version of eslint which is why we had the version from kcd-scripts in our package.json to begin with.

I think you're right. Let me add it to the PR and we'll go from there, as it's (annoyingly) not throwing said error in my env.

@mpeyper
Copy link
Member

Also on the subject of#729, my POV is move them to devDeps and not include them as peer deps, this is what other libraries typically do from my experience. Happy to be told otherwise though.

I've just done a quick whip around of a few big react libraries written in TS that I'm aware of and it does appear to be the case that most just list the types asdevDependencies and nothing else. I'm not sure if that's because their API does not expose anything from these types (although I think it's unlikelynone of them expose aReactNode orReactElement somewhere) or if they just accept that there will be a type error when it's not installed.

The only one I could find doing it the way I'm proposing itshould be done ismaterial-ui. I'm still of the opinion that this is the thebetter way to handle it as it will give a peer dependency warning if the wrong version is installed, but not complain if the type dependencies are not required (e.g. non-TS users).

I've checked out generated type definitions and only@types/react would need to be apeerDependecy (and optional inpeerDependenciesMeta).react-dom andreact-test-renderer could just move todevDependencies as they are not referenced at all in the output.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

What's the reason @types are being added as dependencies as opposed to peerDependencies?
2 participants
@joshuaellis@mpeyper

[8]ページ先頭

©2009-2025 Movatter.jp