fix(react-intl): add typescript as peerDependency, fix #2066 #2067
Conversation
|
Hmm, this might have been better released as a breaking change, instead of in the patch release 5.8.1. Having routinely bumped up the version of
|
|
Hmm why are you required to? |
|
I'm required to add Additionally, when I do add it, I'm required to use a version within the specific range of "3.8 || 4". This is currently impossible for me, because another package I'm using lists Concretely, after upgrading |
|
Yeah you can ignore the warning |
This isn't necessarily |
I'd prefer not to. The warning is there for a reason: We've found our project to be much more stable when we don't just ignore or paper over warnings, including peer-dependency warnings. One of our highest priorities is making it easy for new contributors to get started—and letting lots of warnings accumulate in a routine setup step like |
|
Up to you |
The upgrade guides for v4 [1] and v5 [2] are pretty short and simple, so, might as well jump to the latest all at once. Well, not quite the latest -- with 5.8.1 and following, `react-intl` has `typescript` as a peer dependency, with the range "3.8 || 4". Unfortunately, this range is disjoint with another peer-dep range that caused us to add `typescript` in the first place; see 78daeb6. (See also formatjs/formatjs#2067 (comment) for a `react-intl` maintainer's flippant response to the idea of using peer dependencies correctly.) So, use 5.8.0 precisely. With v4 (and continuing into v5), we get a big stroke of luck for the libdef. There's a single TypeScript file that contains all the necessary types [3], so we only need to run Flowgen once, on that one file. This saves a lot of the kind of work we had to do for v3. The v4 declared breaking changes don't apply to us. One v5 breaking change applies to us: - `FormattedMessage` render prop is no longer variadic. We've only ever cared about the first argument passed to the render prop, so, grab just that one, to minimally handle the API change. In fact, it seems like a bug that we're not paying attention to any others that are passed. But it looks like no others are being passed, currently; we haven't explicitly made that happen. But we'll stop using this confusing pattern in an upcoming commit; see discussion [4]. [1] https://formatjs.io/docs/react-intl/upgrade-guide-4x [2] https://formatjs.io/docs/react-intl/upgrade-guide-5x [3] except for React, which is normal, and which we'll address to some extent in the next commit. [4] zulip#4222 (comment)
The upgrade guides for v4 [1] and v5 [2] are pretty short and simple, so, might as well jump to the latest all at once. Well, not quite the latest -- with 5.8.1 and following, `react-intl` has `typescript` as a peer dependency, with the range "3.8 || 4". Unfortunately, this range is disjoint with another peer-dep range that caused us to add `typescript` in the first place; see 78daeb6. (See also formatjs/formatjs#2067 (comment) for a `react-intl` maintainer's flippant response to the idea of using peer dependencies correctly.) So, use 5.8.0 precisely. With v4 (and continuing into v5), we get a big stroke of luck for the libdef. There's a single TypeScript file that contains all the necessary types [3], so we only need to run Flowgen once, on that one file. This saves a lot of the kind of work we had to do for v3. The v4 declared breaking changes don't apply to us. One v5 breaking change applies to us: - `FormattedMessage` render prop is no longer variadic. We've only ever cared about the first argument passed to the render prop, so, grab just that one, to minimally handle the API change. In fact, it seems like a bug that we're not paying attention to any others that are passed. But it looks like no others are being passed, currently; we haven't explicitly made that happen. But we'll stop using this confusing pattern in an upcoming commit; see discussion [4]. [1] https://formatjs.io/docs/react-intl/upgrade-guide-4x [2] https://formatjs.io/docs/react-intl/upgrade-guide-5x [3] except for React, which is normal, and which we'll address to some extent in the next commit. [4] zulip#4222 (comment)
|
Or rather let's put it this way: do you have an alternative solution? Bumping major version doesn't solve this problem. |
I appreciate it when packages wait for a major-version release to make a public API change. Is this where you disagree? Or would you say that introducing peer-dependency constraints, or dropping previously valid versions from a peer dependency's version range, are not public API changes? Above, I linked to a discussion of that question: semver/semver#502 (comment) |
|
Those are not public API changes. We don't bump major when we drop node 8, or react 16.3, or old CLDR dataset |
Since you're reasoning from |
|
Yes you should always pin to a specific version instead of taking ranges and run you test suite when you upgrade. |
The upgrade guides for v4 [1] and v5 [2] are pretty short and simple, so, might as well jump to the latest (currently 5.8.6) all at once. We restrict the version range to that single version (instead of using a caret or a tilde), since `react-intl` doesn't invite its users to expect breaking changes to be handled well [3]. We also change our useless `typescript` dev-dependency's version to a range that's in the intersection of the range `react-intl` gives for it, and the range we found in 78daeb6. With v4 (and continuing into v5), we get a big stroke of luck for the libdef. There's a single TypeScript file that contains all the necessary types [4], so we only need to run Flowgen once, on that one file. This saves a lot of the kind of work we had to do for v3. The v4 declared breaking changes don't apply to us. One v5 breaking change applies to us: - `FormattedMessage` render prop is no longer variadic. We've only ever cared about the first argument passed to the render prop, so, grab just that one, to minimally handle the API change. In fact, it seems like a bug that we're not paying attention to any others that are passed. But it looks like no others are being passed, currently; we haven't explicitly made that happen. But we'll stop using this confusing pattern in an upcoming commit; see discussion [5]. [1] https://formatjs.io/docs/react-intl/upgrade-guide-4x [2] https://formatjs.io/docs/react-intl/upgrade-guide-5x [3] formatjs/formatjs#2067 (comment) [4] except for React, which is normal, and which we'll address to some extent in the next commit. [5] zulip#4222 (comment)
The upgrade guides for v4 [1] and v5 [2] are pretty short and simple, so, might as well jump to the latest (currently 5.8.6) all at once. We restrict the version range to that single version (instead of using a caret or a tilde), since `react-intl` doesn't invite its users to expect breaking changes to be handled well [3]. We also change our useless `typescript` dev-dependency's version to a range that's in the intersection of the range `react-intl` gives for it, and the range we found in 78daeb6. With v4 (and continuing into v5), we get a big stroke of luck for the libdef. There's a single TypeScript file that contains all the necessary types [4], so we only need to run Flowgen once, on that one file. This saves a lot of the kind of work we had to do for v3. The v4 declared breaking changes don't apply to us. One v5 breaking change applies to us: - `FormattedMessage` render prop is no longer variadic. We've only ever cared about the first argument passed to the render prop, so, grab just that one, to minimally handle the API change. In fact, it seems like a bug that we're not paying attention to any others that are passed. But it looks like no others are being passed, currently; we haven't explicitly made that happen. But we'll stop using this confusing pattern in an upcoming commit; see discussion [5]. [1] https://formatjs.io/docs/react-intl/upgrade-guide-4x [2] https://formatjs.io/docs/react-intl/upgrade-guide-5x [3] formatjs/formatjs#2067 (comment) [4] except for React, which is normal, and which we'll address to some extent in the next commit. [5] zulip#4222 (comment)
Like in 8038528, we restrict the version range to that single version (instead of using a caret or a tilde), since `react-intl` doesn't invite users to expect them to handle breaking changes well [1]. There's no particular change we're interested in right now; we just want to upgrade for the sake of upgrading. Nothing stands out in the changelog [2] as being a breaking change, but we don't really trust the package, as mentioned, so the easiest way to discover any will probably be to have it out in a beta for a little while. [1] formatjs/formatjs#2067 (comment). [2] https://github.com/formatjs/formatjs/blob/main/packages/react-intl/CHANGELOG.md
Like in 8038528, we restrict the version range to that single version (instead of using a caret or a tilde), since `react-intl` doesn't invite users to expect them to handle breaking changes well [1]. There's no particular change we're interested in right now; we just want to upgrade for the sake of upgrading. Nothing stands out in the changelog [2] as being a breaking change, but we don't really trust the package, as mentioned, so the easiest way to discover any will probably be to have it out in a beta for a little while. [1] formatjs/formatjs#2067 (comment). [2] https://github.com/formatjs/formatjs/blob/main/packages/react-intl/CHANGELOG.md
Like in 8038528, we restrict the version range to that single version (instead of using a caret or a tilde), since `react-intl` doesn't invite users to expect them to handle breaking changes well [1]. There's no particular change we're interested in right now; we just want to upgrade for the sake of upgrading. Nothing stands out in the changelog [2] as being a breaking change, but we don't really trust the package, as mentioned, so the easiest way to discover any will probably be to have it out in a beta for a little while. I did test switching to a different language, and things didn't seem to break when I did that. [1] formatjs/formatjs#2067 (comment). [2] https://github.com/formatjs/formatjs/blob/main/packages/react-intl/CHANGELOG.md
Like in 8038528, we restrict the version range to that single version (instead of using a caret or a tilde), since `react-intl` doesn't invite users to expect them to handle breaking changes well [1]. There's no particular change we're interested in right now; we just want to upgrade for the sake of upgrading. Nothing stands out in the changelog [2] as being a breaking change, but we don't really trust the package, as mentioned, so the easiest way to discover any will probably be to have it out in a beta for a little while. I did test switching to a different language, and things didn't seem to break when I did that. [1] formatjs/formatjs#2067 (comment). [2] https://github.com/formatjs/formatjs/blob/main/packages/react-intl/CHANGELOG.md
No description provided.