Make module API more functional, mitigate conflicting config options

[kachkaev]

Following the discussions in https://github.com/isaachinman/next-i18next/issues/33, https://github.com/isaachinman/next-i18next/issues/32 and https://github.com/isaachinman/next-i18next/issues/16, I crafted a couple of examples that demonstrate how the usage of next-i18next could look like if the API was made more functional and less dependent on the config.

https://github.com/kachkaev/next-i18n/tree/master/examples 👀

I don't have enough time before the weekend to describe all design justifications – let the examples speak for themselves. Here a few highlights though:

1. There are multiple npm packages instead just one entry point, which makes the browser bundle obese.
   • Need an SPA in a language derived from user preferences?
     npm install @next-i18next/based-on-user @next-i18next/server-express
   • Having multiple hosts (e.g. subdomains) and using server Koa? npm install @next-i18next/based-on-host @next-i18next/server-koa
   • Changed your mind?
     Uninstall one module and install another one, modify module name in just one require().
2. Exports in i18n.js are typescript/es6 compatible (using export const xxx facilitates type checking and autocompletion in editors).
3. Explicit mentioning of withNamepaces and Trans inside i18n.js reminds users to not import them directly from react-next18n, which will cause issues.
4. A bit more boilerplate to copy-paste in i18n.js makes things more transparent and helps users understand what they can do with the library. No hidden object parts to re-use, as it is now (the current approach is also not typescript-compatible).
5. The server-side config such as the list of hosts sits inside server.js, not i18n.js. This means that no server-side files like config.js will be imported into i18n.js and so no IP will be accidentally leaked. pathToTranslations etc. also lives in server.js as this is irrelevant for the client.
6. The list of available languages, language to host mappings and other things potentially needed in some use cases will be available to the client via __NEXT_DATA__ – no need to configure the client because of that.
7. No need for conflicting config options like browserLanguageDetection, localeSubpaths or localeSubdomains. Config options related to the middeware are passed on middleware creation inside server.js. The functional nature of the API makes it possible to type-check more things, e.g. the lack of of hostByLanguage in server.ts when @next-i18next/based-on-host is used in i18n.js ✨
8. Switching languages is done using <Link language="xx" href="/page" />, where href is optional (current path by default).
9. Components don't have to import stuff from i18next and react-i18next, which will not be in the list of direct project dependencies (doing so would make eslnt / tslint unhappy). Basically all the i18n bits and pieces the app may need are vividly declared in i18n.js and are imported from there.
10. i18next and react-i18next are completely abstracted away in the eyes of a newcomer (people will be still encouraged to use https://locize.com/). Getting into details of i18next is only needed when you need some custom plugin like ICU.
11. (Updated) Re. next export see https://github.com/isaachinman/next-i18next/issues/42#issuecomment-447486698 and https://github.com/isaachinman/next-i18next/issues/42#issuecomment-447488552

You'll probably notice that the examples refer to npm modules as @next-i18n/*, not @next-i18next/* – it's me leaving myself an opportunity to make an alternative Next / I18next integration if this proposal is rejected 😅 I'd love to collaborate on next-18next, but if discussing the ideas ends up being challenging, I'm not sure I'll have enough mental energy to work on the PRs. Let's be polite to each other and thus attract more collaborators into the pool. ✌️

WDYT about the proposed API layout in general? What are the flaws?

[isaachinman]

Thanks, this is interesting work. I would consider some of this for a 1.x release, but currently core problems like #40 are taking top priority.

[kachkaev]

The proposed API layout could easily support next export too. Here is a modified example from Next's README:

// next.config.js
const { internationalizePathMap } = require('@next-i18next/based-on-path');
module.exports = {
  exportPathMap: async function (defaultPathMap) {
    return internationalizePathMap({
      '/': { page: '/' },
      '/about': { page: '/about' },
      '/readme.md': { page: '/readme' },
      '/p/hello-nextjs': { page: '/post', query: { title: 'hello-nextjs' } },
      '/p/learn-nextjs': { page: '/post', query: { title: 'learn-nextjs' } },
      '/p/deploy-nextjs': { page: '/post', query: { title: 'deploy-nextjs' } }
    }, {
        fallbackLanguage: 'en', // we don't have access to i18n.js here, so we need to remind Next what default language we use
        availableLanguages: ['..', '..'], // the list is automatically derived by default from translation files
        pathToTranslations: '...', // defaults to static/translations, just like in the middleware in server.js; only needed when availableLanguages are not defined (this can be linted in TypeScript)
    })
  }
}

Or in most cases:

// next.config.js
const { internationalizePathMap } = require('@next-i18next/based-on-path');
module.exports = {
  exportPathMap: async function (defaultPathMap) {
    return internationalizePathMap(defaultPathMap, {
        fallbackLanguage: 'en',
        availableLanguages: ['en', 'de'],
    })
  }
}

Internationalising next export does not make sense for @next-i18next/based-on-user (browser langauge detector), but can work pretty well for @next-i18next/based-on-path. It is also potentially compatible with @next-i18next/based-on-host – in this case either the host name or the language name can be prepended to output file paths. Despite a possible dependency to fs in internationalizePathMap, this module should not spill over to the browser as long as @next-i18next/based-on-(path|host) provide a tree-shakeable esnext version, which next build can cope with.

[isaachinman]

To be clear, the proposed changes are not related to support of next export - this is possible regardless. There's plenty else to focus on with this package, and supporting static exports isn't at the top of my list at the moment. It's certainly a "nice to have", though.

[kachkaev]

I'm just sharing my thoughts for the future – no attempts to re-prioritise your work whatsoever @isaachinman 😉

AFAIK, next export simply takes a built version of next, kind of starts the server and 'visits' all pages in the given path map. The result is dumped to a filesystem, that's it. There is a chance that I misunderstand the process, sorry if so.

UPD: looks like there is a problem with next export + custom server indeed: https://spectrum.chat/next-js/general/custom-server-with-static-html-export~1aabfe4c-7f93-4c94-9db7-a6223f441900. Nevertheless, this is something that's still possible to overcome with the right design (all we need is to stuff Next with translations during the export).

UPD2: Actually, this can be just

const { internationalizePathMap } = require('@next-i18next/static-export');

Injecting the translation data into query should work regardless of the choice for @next-i18next/based-on-(user|path|host). The requirements to the output are the same in all cases: each language gets its own sub-path at the root of the export folder.

[isaachinman]

Number 1 priority remains tree walking and context issues. Until we come to a solid solution for that, this project is at a stand still.

Thanks for your proposals, we will revisit this in the (near, hopefully) future.

[isaachinman]

So, this a nice wish list, but there some problems, and some things would just be prohibitively hard to achieve. When responding, it would be helpful to use the numbers below so that it's clear which feature request we are discussing.

1. Splitting into multiple npm packages: this will not happen anytime soon. This requires significant infrastructure changes. This project is still young, and it's not yet clear exactly how users will want to use it, or which directly the functionality will head. In short, I believe this is premature. Instead, code should be excluded from the client bundle based on config options.
2. Use export const xxx for typing: the core reason we're exporting a class constructor and returning methods inside of it is that we need to take the user-provided config, init an i18n instance, and then share that instance with all methods. There is the possibility for a functional approach where the user handles the i18n instance themselves and calls a bunch of createXXX functions, but this involves a lot of unnecessary boilerplate for users, and also increases the chances of things breaking in user land. If there is a proposal that keeps initialisation to a one-liner, I am happy to hear it.
3. Explicit mentioning... more boilerplate: this will not happen. If the API is not clear, the docs should be improved. Adding boilerplate is not an appropriate solution.
4. Server side config: not sure what this one is. We do switch over client/server when creating the config, and I don't understand what pathToTranslations is. Please let me know if I've misunderstood.
5. Available in __NEXT_DATA__: if I understand this request correctly, you want to add things to pageProps? That's not a problem, of course.
6. Conflicting config options, browserLanguageDetection, localeSubpaths: again, we're not going to split into multiple packages anytime soon, so any desired fix will need to happen given the current structure. Also I'm not sure if you missed this, but both browserLanguageDetection and localeSubpaths depend on client side code. It's not a middleware.
7. Switching languages is done using : why? I suppose we could call i18n.changeLanguage if there's a language prop on the Link component, but this doesn't feel correct to me. Changing languages should be imperative. The Link component is for routing, not changing user language.
8. Components don't have to import stuff from i18next and react-i18next: can you explain further? Don't understand this one.
9. i18next and react-i18next are completely abstracted away: can you explain further? That is rather the entire point of this project, and I feel we're doing a pretty good job already. Users will of course need to dive into i18next and react-i18next if they want to do non-default stuff.
10. Next export: we've moved away from tree-walking, and next export works just fine in next-i18next. However, it's never going to be a main focus of this project, as SSR is a core part of the approach.

In general this is not one proposal which will be accepted or rejected as a whole, but rather approximately ten proposals which will be appraised individually. Each individual proposal should (if we proceed with it) be accompanied by its own issue and PR.

[kachkaev]

Thanks for sharing your thoughts @isaachinman – I'll respond when I have more time to think on your comments. Meanwhile, could you please update the numbering in your reply so that it matched the items in my original list? I just replaced UL with OL there. Next export becomes number 11 instead of 10.

No rush with issue I guess. It's all about about the strategy, so I'm fine with a slow conversation pace.

[isaachinman]

@kachkaev Do you feel any/all of these points are still valid?

[kachkaev]

Hi @isaachinman! I'll be happy to look at the current state of things over the weekend 😉 Could you please sync the points in your comment with the list I shared originally? This is just so that everyone could easily refer to smth.

[lipoolock]

Hi @kachkaev Does this implementation of Language switching is closed to be released : https://github.com/kachkaev/next-i18n/blob/master/examples/basic/components/Menu.jsx ? I'm particularly concerned by this and the solution with Button doesn't fit my expectations (I need to navigate on language change to make some API calls in getInitialProps, not only fallback on locale files)

Thanks ;)

[isaachinman]

I'm going to close this now, as usage of next-i18next and the API therein has settled and become fairly stable.

That being said, the API will probably completely change when this package is rewritten as a first-class NextJs plugin.

If anyone feels as though any of the suggestions mentioned here warrant more discussion, please open individual issues for them.