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

Explicitly Semantic Versioning #3099

Open
Ryan-Dia opened this issue Aug 19, 2023 · 3 comments
Open

Explicitly Semantic Versioning #3099

Ryan-Dia opened this issue Aug 19, 2023 · 3 comments

Comments

@Ryan-Dia
Copy link

Ryan-Dia commented Aug 19, 2023

The problem
The current package.json for @emotion/react and @emotion/styled is as follows:

"peerDependencies": {
    "react": ">=16.8.0"
  },
  "peerDependenciesMeta": {
    "@types/react": {
      "optional": true
    }
  },

This code is ultimately equivalent to the following code.

"peerDependencies": {
    "react": ">=16.8.0",
    "@types/react": "*"
  },
  "peerDependenciesMeta": {
    "@types/react": {
      "optional": true
    }
  },
image

Upon verification, I learned that the addition of peerDependenciesMeta was due to errors occurring in v10.

If using Meta, I believe it would be even better to explicitly align the version of react within peerDependencies. This is why I decided to write this message. Since you've already set React to be >=16.8.0, specifying @types/react >= 16.8.0 seems safer and more suitable for common practices than using @types/react "*" indiscriminately.

Proposed solution

Explicitly adding

@emotion/react

"peerDependencies": {
    "react": ">=16.8.0",
    "@types/react": ">=16.8.0"
  },
  "peerDependenciesMeta": {
    "@types/react": {
      "optional": true
    }
  },

@emotion/styled

"peerDependencies": {
    "@emotion/react": "^11.0.0-rc.0",
    "react": ">=16.8.0",
    "@types/react": ">=16.8.0"
  },
  "peerDependenciesMeta": {
    "@types/react": {
      "optional": true
    }
  },

Alternative solutions

Additional context

#3100

@Andarist
Copy link
Member

The problem is that we dont want installations to trigger missing peer dep errors in package managers that are not aware of peerDependenciesMeta. In those this peer dep would be treated as a required one

@Ryan-Dia
Copy link
Author

Ryan-Dia commented Aug 19, 2023

I completely understand. However, this update took place around three years ago, approximately in July 2020.

peerDependenciesMeta was introduced around the end of 2018 and gradually adopted starting in 2019. By the time the restrictions on changes were enforced around April 2020, as you mentioned, there were likely many users still utilizing package versions that weren't compatible. While it was a valid consideration at that time, in August 2023, I believe the number of such users would be extremely minimal.

Since peerDependenciesMeta was introduced starting from npm v7, users on versions earlier than v7 might indeed find it a necessary requirement, as you mentioned. However, node.js 14, which can support npm versions below v7, has reached its End-of-Life status. Additionally, node.js 16, which mandates a minimum of npm v7, is also nearing its End-of-Life phase.

Yarn] users generally operate on versions older than 1.13 from about four years ago, and similarly, pnpm users are on versions prior to 3.2.0 from around the same time, both of which do not support peerDependenciesMeta

Furthermore, packages like next.js, style-component, jest, and others have also been employing both peerDependencies and peerDependenciesMeta.

As you mentioned, it's important not to trigger peer dependency errors using package managers that don't recognize peerDependenciesMeta. However, aligning the semantic versioning of both react and @types/react is also crucial, in my opinion. This suggestion doesn't aim to be ahead of other packages, but rather to propose it since other packages are already utilizing both peerDependencies and peerDependenciesMeta.

I respect either approach.

@Andarist
Copy link
Member

If this would be a different peer dep than @types/react I would also add it to peerDependencies despite it being optional. It feels to me though that @types/react are somewhat special here. This is a types-only package and I have a hard time imagining that we should be responsible for restricting its version. Yes, it is crucial for versions of react and @types/react to be compatible with each other. I feel though that the responsibility for matching them is on the user's side - if they won't match then they likely have bigger problems than a mismatch with missing our implied requirements in Emotion. Since we specify the range of compatible React versions - I feel that also implies what @types/react versions we are compatible with (implicitly, yes - but still). Many projects don't add @types/react to their dependencies at all (despite the fact that they do depend on them) and things usually still work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants