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: Add @types/react-dom as a direct dependency #1001

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Dec 3, 2021

@types/react-dom is used in types/index.d.ts, which is an external interface, and thus needs to be declared as a regular rather than a dev dependency so that it is picked up by users of @testing-library/react.

What:

This fixes #1000.

Why:

@types/react-dom is used in types/index.d.ts, which is an external interface, and thus needs to be declared as a regular rather than a dev dependency so that it is picked up by users of @testing-library/react.

How:

Declare @types/react-dom as a regular rather than a dev dependency.

Checklist:

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

`@types/react-dom` is used in `types/index.d.ts`, which is an external interface, and thus needs to be declared as a regular rather than a dev dependency so that it is picked up by users of `@testing-library/react`.
@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 3, 2021

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 23e641f:

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

@MatanBobi
Copy link
Member

MatanBobi commented Dec 5, 2021

Hi @fmeum! Thanks for taking the time to work on this one.
I'm not sure how and if we should tackle this one.
The reason is that we don't want to add 3rd party types into our bundle since not all of our users are using TypeScript.
The second reason is that having versioned types isn't good for us because we support multiple React versions (same as we have peer deps defined as react: * and react-dom: *)
I think that once people do have React and React-DOM and are using TypeScript, they will also have the types installed.
I'm not too familiar with the build system that you're using, but do you have @types/react-dom installed and it still gives you the error?

@fmeum
Copy link
Contributor Author

fmeum commented Dec 5, 2021

I'm not very familiar with the resolution mechanism for dependencies, but yarn (see e.g. yarnpkg/yarn#3951) seems to recommend a peer dependency on * or a suitable version range for @types. Would that help address your second point? I could adapt the PR to use that.

Regarding the build system: Yes, @types/react-dom is installed, but the build system ensures that only packages that declare a (non-dev) dependeny on it in their package.json get to see it. I don't want to break or complicate the build with the popular build systems though, so if there is no good solution to this, I will use a manual workaround.

package.json Outdated Show resolved Hide resolved
@eps1lon eps1lon changed the title Make @types/react-dom a regular dependency fix: Add @types/react-dom as a direct dependency Dec 6, 2021
@eps1lon eps1lon added the bug Something isn't working label Dec 6, 2021
@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #1001 (5a88092) into main (8f17a2b) will not change coverage.
The diff coverage is n/a.

❗ Current head 5a88092 differs from pull request most recent head 33e2803. Consider uploading reports for the commit 33e2803 to get more accurate results

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #1001   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          140       142    +2     
  Branches        26        28    +2     
=========================================
+ Hits           140       142    +2     
Impacted Files Coverage Δ
src/pure.js 100.00% <0.00%> (ø)
src/act-compat.js 100.00% <0.00%> (ø)

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 8f17a2b...33e2803. Read the comment docs.

Co-authored-by: Sebastian Silbermann <[email protected]>
@fmeum
Copy link
Contributor Author

fmeum commented Dec 6, 2021

@eps1lon Accepted your suggestion, thanks.

@fmeum
Copy link
Contributor Author

fmeum commented Feb 12, 2022

@eps1lon Friendly ping

@eps1lon
Copy link
Member

eps1lon commented Feb 15, 2022

@eps1lon Friendly ping

Could you give permissions to maintainers to edit this PR (should be on th right side next to labels, etc)? Otherwise I can't force GH actions to run on this PR.

@fmeum
Copy link
Contributor Author

fmeum commented Feb 15, 2022

@eps1lon Friendly ping

Could you give permissions to maintainers to edit this PR (should be on th right side next to labels, etc)? Otherwise I can't force GH actions to run on this PR.

I had it enabled. Just disabled and reenabled it to make sure it works.

@eps1lon
Copy link
Member

eps1lon commented Feb 15, 2022

@eps1lon Friendly ping

Could you give permissions to maintainers to edit this PR (should be on th right side next to labels, etc)? Otherwise I can't force GH actions to run on this PR.

I had it enabled. Just disabled and reenabled it to make sure it works.

Mkay. I guess it's GitHub codespaces being useless and not allowing me to push.

@eps1lon eps1lon merged commit 149d9a9 into testing-library:main Feb 15, 2022
@eps1lon
Copy link
Member

eps1lon commented Feb 15, 2022

@all-contributors add @fmeum for code, bug

@allcontributors
Copy link
Contributor

@eps1lon

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

@github-actions
Copy link

🎉 This PR is included in version 12.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 13.0.0-alpha.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

cbush pushed a commit to mongodb/docs-realm that referenced this pull request Mar 15, 2023
<h3>Snyk has created this PR to upgrade @testing-library/react from
12.1.3 to 12.1.5.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.
<hr/>

- The recommended version is **2 versions** ahead of your current
version.
- The recommended version was released **a year ago**, on 2022-04-11.


<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>@testing-library/react</b></summary>
    <ul>
      <li>
<b>12.1.5</b> - <a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/releases/tag/v12.1.5">2022-04-11</a></br><h2><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/compare/v12.1.4...v12.1.5">12.1.5</a>
(2022-04-11)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>Only supports React &lt; 18 (<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/issues/1041"
data-hovercard-type="pull_request"
data-hovercard-url="/testing-library/react-testing-library/pull/1041/hovercard">#1041</a>)
(<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/9e2b5dbb4632799ae38f1341cb79ef35d1bd6652">9e2b5db</a>)</li>
</ul>
      </li>
      <li>
<b>12.1.4</b> - <a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/releases/tag/v12.1.4">2022-03-09</a></br><h2><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/compare/v12.1.3...v12.1.4">12.1.4</a>
(2022-03-09)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>Match runtime type of baseElement in TypeScript types (<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/issues/1023"
data-hovercard-type="pull_request"
data-hovercard-url="/testing-library/react-testing-library/pull/1023/hovercard">#1023</a>)
(<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/96ed8dafa5d02add2168a3da65d1cc0ffe6d6d1f">96ed8da</a>)</li>
</ul>
      </li>
      <li>
<b>12.1.3</b> - <a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/releases/tag/v12.1.3">2022-02-15</a></br><h2><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/compare/v12.1.2...v12.1.3">12.1.3</a>
(2022-02-15)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>Add <code>@ types/react-dom</code> as a direct dependency (<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/issues/1001"
data-hovercard-type="pull_request"
data-hovercard-url="/testing-library/react-testing-library/pull/1001/hovercard">#1001</a>)
(<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/149d9a9af3addeb6c49696867b05b87afe0d0b3c">149d9a9</a>)</li>
</ul>
      </li>
    </ul>
from <a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/releases">@testing-library/react
GitHub release notes</a>
  </details>
</details>


<details>
  <summary><b>Commit messages</b></summary>
  </br>
  <details>
    <summary>Package name: <b>@testing-library/react</b></summary>
    <ul>
<li><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/646129b59659e2f3509a6fff606a9871b2a68a9c">646129b</a>
chore: Run release from 12.x branch (#1044)</li>
<li><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/9e2b5dbb4632799ae38f1341cb79ef35d1bd6652">9e2b5db</a>
fix: Only supports React &lt; 18 (#1041)</li>
<li><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/0c4aabe0da1587754229f7614a2ddfdaddd0b1aa">0c4aabe</a>
chore: Fix failing codesandbox/ci (#1026)</li>
<li><a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/commit/96ed8dafa5d02add2168a3da65d1cc0ffe6d6d1f">96ed8da</a>
fix: Match runtime type of baseElement in TypeScript types (#1023)</li>
    </ul>

<a
href="https://snyk.io/redirect/github/testing-library/react-testing-library/compare/b0f9d9741205c54836bf82b76b86ec001a8c0e1f...646129b59659e2f3509a6fff606a9871b2a68a9c">Compare</a>
  </details>
</details>
<hr/>

**Note:** *You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs.*

For more information: <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiI2ZDcwZTlkNC05OGFjLTRkNDktYjc0Ny1mY2NlMDI4MTBhMjEiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjZkNzBlOWQ0LTk4YWMtNGQ0OS1iNzQ3LWZjY2UwMjgxMGEyMSJ9fQ=="
width="0" height="0"/>

🧐 [View latest project
report](https://app.snyk.io/org/sandbox-2ba/project/9043c51f-3f0d-45c6-8455-b658274f2872?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)

🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/sandbox-2ba/project/9043c51f-3f0d-45c6-8455-b658274f2872/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)

🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/sandbox-2ba/project/9043c51f-3f0d-45c6-8455-b658274f2872/settings/integration?pkg&#x3D;@testing-library/react&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

<!---
(snyk:metadata:{"prId":"6d70e9d4-98ac-4d49-b747-fcce02810a21","prPublicId":"6d70e9d4-98ac-4d49-b747-fcce02810a21","dependencies":[{"name":"@testing-library/react","from":"12.1.3","to":"12.1.5"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/sandbox-2ba/project/9043c51f-3f0d-45c6-8455-b658274f2872?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"9043c51f-3f0d-45c6-8455-b658274f2872","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":2,"publishedDate":"2022-04-11T20:14:35.067Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]})
--->

---------

Co-authored-by: snyk-bot <[email protected]>
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.

@types/react-dom incorrectly declared as a devDependency
3 participants