Use peer dependencies

[capellini]

The discussion of peer dependencies in #129 got me thinking - what do you think about moving react-i18next, i18next (and perhaps other associated i18next packages), and react to dev dependencies and also including them as peer dependencies (much like the next-i18next package does with i18next and react)? It would help communicate minimum compatible versions of each package and would help us avoid yarn warnings like this one:

'warning "next-i18next > react-i18next@9.0.9" has incorrect peer dependency "i18next@>= 14.0.1".

In this case, our package.json specifies i18next@^13.1.3, but the next-i18next package.json specifies react-i18next@^9.0.4 (currently v9.0.9) which has a peer dependency of i18next@^14.0.1. I think that we're going to constantly run into these incorrect peer dependency warnings unless we just specify next-i18next as a peer dependency (with the lowest compatible version number). Then users could install our package alongside the other peer dependencies when they want to use next-i18next.

We would still have to keep the dev dependencies up to date, but the warnings would not show up for our users and I think it communicates more effectively that these packages are to be installed with our package and we only need to install them in our package if we're doing some sort of development (e.g. our e2e tests).

[isaachinman]

Yes, that sounds very sensible to me. Let's do that.

[capellini]

Per #129 - also include core-js and @babel/polyfill as peer dependencies.

[isaachinman]

Fixed with 9c23e89. I don't think core-js or @babel/polyfill belong in peer deps.

[capellini]

If i18next and react are peerDependencies, then they shouldn't also be dependencies. They should be moved to devDependencies.

Should react-i18next also be a peer dependency?

[capellini]

If we don't enforce core-js as a peerDependency, don't we run the risk of having #129 happening again? My understanding is that, with the way that babel polyfills work, the core-js that matters is one that is installed in the root node_modules folder of whatever project is using next-i18next, not necessarily what we have installed in our node_modules folder.

[kachkaev]

Possibly related: #152

[isaachinman]

@kachkaev Yes it's related. I just really don't like the idea of having to put core-js as a peer dep... Need to think on this one. Maybe we should move away from Babel in general.

[kachkaev]

I guess that removing @babel/polyfill from .babelrc and core-js from dependencies / devDependencies should be it. People should polyfill things only one per app IMO, which is most efficient and side-effect-free. This does not mean that we can't help devs with advising on what to put into their app's .babelrc if the want to support IE11 / other old browsers with no I18n api etc.

[isaachinman]

Removed polyfill with 4bf9d37.

And no, react-i18next shouldn't be a peer dep as our users never need to touch it.

[capellini]

If i18next and react are peerDependencies, then they shouldn't also be dependencies. They should be moved to devDependencies.

[isaachinman]

@capellini Ah, sorry - missed that. Fixed with f35cc48.

[capellini]

No worries. Thanks!

[kachkaev]

I might be wrong, but it seems that moving i18next to devDependencies means that setup changes from

yarn add next-i18next

to

yarn add next-i18next i18next

This library has react-i18next as a direct dependency, but that library does not directly depend on i18next (see its package.json).

[isaachinman]

@kachkaev That's what a peer dependency is. Please refer to the README for updated installation instructions.

[kachkaev]

I believe I understand the purpose of devDependencies and I see why react and next are in this category (any Next.js project does not make sense without them). Contrary to those, i18next and react-i18next appear to be a part of I18n integration, which next-i18next brings in. As a project owner who wants to get I18n sorted in my existing Next.js app, I'd rather install just next-i18next and start working with it right away, without caring about implementation details. That's what Next plugins like @zeit/next-css and @zeit/next-typescript do – they pull all the deps that they need without my involvement.

Using i18next as a peer dependency feels more like an advanced way of adding I18n to the app, which expected to involve imports both from react-i18next and i18next into the app's code.

[isaachinman]

@kachkaev From @capellini:

In this case, our package.json specifies i18next@^13.1.3, but the next-i18next package.json specifies react-i18next@^9.0.4 (currently v9.0.9) which has a peer dependency of i18next@^14.0.1. I think that we're going to constantly run into these incorrect peer dependency warnings

Please discuss if you feel you have another solution.

[kachkaev]

In this case, our package.json specifies i18next@^13.1.3

@capellini do you mean package.json in your app? IMO either both i18next and react-i18next need to belong to it, or none of those. In the first case, both libs will be peerDependencies of this lib and app devs will be able to choose their versions; in the latter case next-i18next will pick the versions for us. I personally prefer the former case, because this reduces the likelihood of next-i18next being incompatible with its tight dependents. See @zeit/next-css's package.json as an example – it automatically picks all the libs that it needs.

Either way, classifying i18next and react-i18next differently feels suboptimal to me.

[capellini]

do you mean package.json in your app?

I mean package.json for next-i18next. Previously, it specified an i18next version that conflicted with the i18next version specified as a peerDependency by react-i18next. Note that, despite my comment, we still have to manage peerDependency version compatibility.

Either way, classifying i18next and react-i18next differently feels suboptimal to me

It felt that way initially to me, as well, but now I'm not so sure. The real question is -- do we expect users to need to install i18next and/or react-18next in their projects? In my case, all of the defaults work well for me, so I don't, so keeping i18next and react-i18next as straight dependencies works for me. However, in advanced use cases, will users need to reach into react-i18next and/or i18next?

Another way to think about it is -- do we consider ourselves a replacement for any package? Or, in other words, would using our package obviate the need to use another? I think that @isaachinman's point is that we consider ourselves a replacement for react-i18next. So instead of users doing this:

yarn add react-i18next i18next

they do this instead:

yarn add next-i18next i18next

As far as they are concerned, react-i18next doesn't need to exist.

If we consider ourselves a 1:1 replacement for react-i18next, then we should mirror what they do and wrap their functionality -- all transparently to the user. Which would mean that i18next is a peerDependency (as it is with react-i18next) and react-i18next is a dependency, as we would never require our users to install it.

However, for advanced use cases, I believe that users may still need to use i18next. In that case, it needs to be a peerDependency so that we're using the same versions.

Yes, we would still need to worry about compatibility with i18next, but I think that we can safely reference react-i18next's package.json to ensure that we are compatible with the version they list in peerDependencies. It seems that the tie to react-i18next versioning is inevitable.

[isaachinman]

@kachkaev Yes, I think I agree with you. The only true peer deps here are next and react.

@capellini To answer your question, I want next-i18next to be a replace for both react-i18next and i18next. In general there is quite a lot of confusion and lack of knowledge in the dev community about i18next and its ecosystem, and this package attempts to hide a lot of that away and allow users to just think about their JSON namespaces and get on with it.

However, for advanced use cases, I believe that users may still need to use i18next

What cases? If there are situations where users need to install and import i18next directly, that probably means we've failed to expose some part of the API.

[isaachinman]

Added i18next back to deps with b986633. Sorry for the back and forth on this one. We're going to move forward with the assumption that next and react are the only peer deps of this package.

[capellini]

What cases? If there are situations where users need to install and import i18next directly, that probably means we've failed to expose some part of the API.

Again, perhaps I'm revealing the extent of my ignorance of i18next. The zero config defaults work for me, so maybe I'm trying to solve a problem that doesn't exist. But does anyone know why react-i18next believes that i18next should be a peerDependency? It seems to me that indicates that there are cases where one would need to install i18next alongside react-i18next. Perhaps the use cases that require them to include it as a peerDependency don't apply to our package?

Either way, the defaults work for me, so I have no objections to react and next being the only peerDependencies. If it becomes a problem at some point in the future, we can always revisit this.

[isaachinman]

But does anyone know why react-i18next believes that i18next should be a peerDependency?

I guess the fact that this question is asked means we're doing a good job here. Take a look at the example implementation from the react-i18next repo. It's much more hands-on, and you definitely touch i18next directly.

[capellini]

Understood. It makes much more sense now that I see the example implementation. Thanks.