Adds @types/react as optional peer deps

[arcanis]

What:

Using Emotion w/ PnP & TypeScript yields an error saying that the result of styled.div isn't a callable JSX construct.

Why:

This is because styled and styled-base are both missing optional peer dependencies on @types/react, so they aren't allowed to access it.

How:

I've added @types/react as optional peer dependencies. No warning will be emitted to consumers using old package managers, or simply not providing @types/react as dependencies.

Checklist:

• Documentation N/A
• Tests - Tested locally using packageExtensions
• Code complete - N/A
• Changeset

[changeset-bot]

🦋 Changeset is good to go

Latest commit: 03e796b

We got this.

This PR includes changesets to release 3 packages

Name	Type
@emotion/react	Patch
@emotion/styled	Patch
@emotion/jest	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (next@ab09e9e). Click here to learn what that means.
The diff coverage is n/a.

Impacted Files	Coverage Δ
packages/create-emotion-server/src/stream.js	100.00% <0.00%> (ø)
packages/primitives/src/test-props.js	100.00% <0.00%> (ø)
packages/emotion-server/src/index.js	100.00% <0.00%> (ø)
packages/css/src/index.js	100.00% <0.00%> (ø)
scripts/babel-preset-emotion-dev/src/index.js	100.00% <0.00%> (ø)
...nt-plugin-emotion/src/rules/import-from-emotion.js	90.90% <0.00%> (ø)
packages/babel-plugin-jsx-pragmatic/src/index.js	94.11% <0.00%> (ø)
packages/unitless/src/index.js	100.00% <0.00%> (ø)
packages/utils/src/index.js	100.00% <0.00%> (ø)
packages/styled-base/src/index.js	100.00% <0.00%> (ø)
... and 71 more

[Andarist]

No warning will be emitted to consumers using old package managers

Isn't it the other way around? Old package managers will emit warnings for this as they don't know what optional peer dependencies are, right?

---

Would you say that @types/* should always be optional peer dependencies (if one, of course, directly depends on them with their own types)?

[arcanis]

Isn't it the other way around? Old package managers will emit warnings for this as they don't know what optional peer dependencies are, right?

No, old package managers won't know what peerDependenciesMeta is, so from their perspective nothing will change. For package managers that support it, they will interpret it as an implicit peer dependency (peerDependencyMeta implies the corresponding peerDependency).

Would you say that @types/* should always be optional peer dependencies (if one, of course, directly depends on them with their own types)?

Yes, unless they are regular dependencies (which I think is a decent choice from a UX perspective, even if it means more dependencies for those who don't use TypeScript).

[Andarist]

No, old package managers won't know what peerDependenciesMeta is, so from their perspective nothing will change.

Right, but from what I understand this will cause warnings because old package managers understand peerDependencies (and @types/react might not be provided by the consumer) but they don't understand peerDependenciesMeta, so it will be a "hard" peer dep (the only kind for them) and thus can result in warnings. I don't care about this much (the extra warning in old managers), just your statement caught my eye and I'm wondering if I understand this correctly.

[arcanis]

My PR only adds peerDependenciesMeta, not peerDependencies, so nothing will change for old package managers.

[Andarist]

Oh - have not noticed that. I suspect it's equivalent to using * just without including it explicitly in peerDeps themselves, right?

[arcanis]

Yep, exactly!

[Andarist]

Is this affecting your personally or did you fix this after somebody's report? I'm not sure if emotion currently is PnP-compliant anyway. From what I recall PnP-compatibility got fixed on next branch and if that's ok with you I would prefer to move this work there (I can handle porting this). We are wrapping up things for the upcoming v11 and would just like to avoid any work on v10 (current master) branch.

[arcanis]

Is this affecting your personally or did you fix this after somebody's report?

I've hit this in a project of mine. That said, I've been able to workaround it using packageExtensions so it's not a blocker for me.

I'm not sure if emotion currently is PnP-compliant anyway.

It is! I have no problem on the v10 line except for this. Since it's honestly a bit tricky to debug (the TS error is rather cryptic, just saying that a JSX element isn't compatible with JSX elements), I think it might be worth backporting, but your call.

[Andarist]

K, I'm gonna move with this in v10. The last thing though - I believe the same should be done for @emotion/core package, right? I can add it myself, just want to get a confirmation on this one. Even if you don't use css prop, and prefer styled API, you still have a peer dep on @emotion/core and it doesn't specify any kind of dependency on @types/react.

Also thanks for mentioning packageExtensions - I didn't know about this and will probably come handy in the future 👍

[codesandbox-ci]

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 03e796b:

Sandbox	Source
Emotion	Configuration

[Andarist]

I've wanted to merge this into v10 branch but in the end, this PR has slipped my attention since April - at this point in time I don't think there is much need to fix this for v10 as we've not received any other reports about this problem. I'm happy to incorporate this change though - I've just merged this into v11 branch.

Thanks for your help on this ❤️