Support to named exports

[gurkerl83]

Is there a way to add the Translate type back. I was testing on 1.0.0-experimental.4 for a while now. The interface gets not exported anymore. Due to the refactoring I do not know if the type still exists anymore. I think the export stopped at 1.0.0-experimental.4.1.

import { Translate } from 'next-translate';

The idea is simple to pass the t-function as an argument in functions and have some type safety in when you call the function.

Also, it would be nice to see the useTranslation hook exported through a named export.

Import now

import useTranslation from 'next-translate/useTranslation';

Named export
import { useTranslation } from 'next-translate';

Thx!

[aralroca]

1.0.0-experimental.4 is quite old. Can you try 1.0.0-canary.2? which is the last prerelease. Not sure if exist this type but we increased the types. If it's definitely not there either, then we can add it, yes!

[gurkerl83]

I did upgrade to 1.0.0-canary.2 in the afternoon, but the type Translate is not there, also useTranslation is not exported. I consume it through.

import useTranslation from 'next-translate/useTranslation';

[gurkerl83]

Looking at the index.d.ts from the sources the Translate interface exists, but the build does not have it. I don`t know the internals how you build the deployment bundle, but its missing there. The builds index.d.ts looks the following, important this is from the esm bundle.

import { ReactElement, ReactNode } from 'react';
export interface TranslationQuery {
    [name: string]: string | number;
}
export interface I18n {
    t(i18nKey: string | TemplateStringsArray, query: TranslationQuery | null | undefined, options: {
        returnObjects?: boolean;
        fallback?: string | string[];
    }): string | object;
    t(i18nKey: string | TemplateStringsArray, query: TranslationQuery | null | undefined): string;
    t(i18nKey: string | TemplateStringsArray): string;
    lang: string;
    loadLocaleFrom?: (language: string, namespace: string) => Promise<I18nDictionary>;
}
export interface I18nProviderProps {
    lang?: string;
    namespaces?: Record<string, I18nDictionary>;
    children?: ReactNode;
    logger?: I18nLogger;
    loadLocaleFrom?: (language: string, namespace: string) => Promise<I18nDictionary>;
}
export interface TransProps {
    i18nKey: string;
    components?: ReactElement[];
    values?: TranslationQuery;
    fallback?: string | string[];
}
export declare type PageValue = string[] | ((context: object) => string[]);
export interface I18nConfig {
    defaultLocale?: string;
    locales?: string[];
    loadLocaleFrom?: (language: string, namespace: string) => Promise<I18nDictionary>;
    pages?: Record<string, PageValue>;
    logger?: I18nLogger;
    loader?: boolean;
    logBuild?: boolean;
}
export interface LoaderConfig extends I18nConfig {
    locale?: string;
    router?: {
        locale: string;
    };
    pathname?: string;
    skipInitialProps?: boolean;
    loaderName?: string;
    isLoader?: boolean;
}
export interface LoggerProps {
    namespace: string;
    i18nKey: string;
}
export interface I18nLogger {
    (context: LoggerProps): void;
}
export interface I18nDictionary {
    [key: string]: unknown;
}
export interface DynamicNamespacesProps {
    dynamic?: (language: string, namespace: string) => Promise<I18nDictionary>;
    namespaces?: string[];
    fallback?: React.ReactNode;
    children?: React.ReactNode;
}

How do you build the lib folder the following fields in package.json are referencing to

"main": "./lib/cjs/index.js",
"module": "./lib/esm/index.js",
"types": "./lib/esm/index.d.ts",

Answering my own question: "outDir": "./lib/esm"

[aralroca]

Also the useTranslation export exists https://www.runpkg.com/[email protected]/useTranslation/package.json ...

How do you build the lib folder the following fields in package.json are referencing to

Just doing yarn build and it executes the follow:

https://github.com/vinissimus/next-translate/blob/72abc8be67f56cd6e98043c61ad2d7012e6f149d/package.json#L58

[aralroca]

what is rare is that the repo examples use the canary.2 version with esm and it works well...! Maybe it would help if you create an example reproducing the error🙏

[gurkerl83]

I think the build is just fine. Translate interface for the t function does not exist anymore, instead it is either

t(
   i18nKey: string | TemplateStringsArray,
   query: TranslationQuery | null | undefined,
   options: { returnObjects?: boolean; fallback?: string | string[] }
): string | object
t(
   i18nKey: string | TemplateStringsArray,
   query: TranslationQuery | null | undefined
): string
t(i18nKey: string | TemplateStringsArray): string

this is just pseudo code not written within an editor.

import { I18n } from 'next-translate';

const translateText = (i18nObject: I18n) => {
   const text = I18n.t(...)
}

[aralroca]

It is likely that by migrating the project to TypeScript in version 1.0 some types have changed, some improved, but others may need to be re-added.

I did upgrade to 1.0.0-canary.2 in the afternoon, but the type Translate is not there, also useTranslation is not exported. I consume it through.

import useTranslation from 'next-translate/useTranslation';

However, I'm worried about this. You should import useTranslation with any problem on 1.0.0-canary.1...

[gurkerl83]

For the useTranslation exposing through a named export I think you have to add useTranslate in the index.tsx file, next where you do

https://github.com/vinissimus/next-translate/blob/72abc8be67f56cd6e98043c61ad2d7012e6f149d/src/index.tsx#L3

import nextTranslate from './plugin'
module.exports = nextTranslate => end of file

The index.tsx will run through typescript, so I think it makes sense to switch the module.exports = for es exports. You already have your tsconfig.json and tsconfig-cjs.json profiles.

Maybe you can try to change

import nextTranslate from './plugin'
export default nextTranslate; => maybe a more fancier default export is possible

export { default as useTranslation } from './useTranslation' => export as named export

...here comes the rest like withTranslation, everything which should be exposed from the library

Does this makes sense?

[aralroca]

Well, useTranslation package should be imported from next-translate/useTranslation:

import useTranslation from 'next-translate/useTranslation'

This is deprecated:

import { useTranslation } from 'next-translate'

In fact, in the latest versions (before 1.0) it is already showing a deprecation warning if used like this:

• https://github.com/vinissimus/next-translate/blob/master/src/index.js#L9-L11

Maybe we could add this clarification in the migration guide from 0.x to 1.0

[gurkerl83]

Reading the comment in index.js makes me thinking, haven`t heard this at all, global imports... Maybe something was misunderstood back then, but doing "named" exports allows "named" imports. This has nothing to do with global imports, I do not really unterstand what those might be in this context.

export { default as useTranslation } from './useTranslation'

allowing imports like

import { useTranslation } from 'next-translate'

any major library like material ui is doing it that way, e.g. https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/index.js

I think the export mechanism in index.js on master is just right, did you experienced any problem then? I mean NextJs can be a beast in terms of the next-config.js file, here it does not support imports (es) but relies still on require (commonjs) import statements but next-translate provides both, so no problem here.

[aralroca]

@gurkerl83 It is open to debate. I prefer that people use it as a separate package to avoid tree-shaking problems, although webpack in principle does tree-shaking I had experienced some problems in the past making the code heavier. I wanted to be strict and that there was only one way to do the imports, but maybe I'm wrong. What are the advantages of allowing these named exports?

Next.js for example also forces to import Router from next/router, link from next/link...

And by "global" I meant that they are all imported from the root rather than from their own package. Perhaps the word "global" is not well used here.

[gurkerl83]

I don't think I can list the pros and cons of Named Exports in detail myself. I have to admit that my arguments are only based on my experience. My primary aspect is the behavior of development environments. Named exports allow an IDE feature Intelli Sense to automatically display possible import variants.

I cannot make any statements about tree shaking. Of course everything is bound to an object but I think Webpack can separate the sub objects attached to the object if there is no mutual entanglement. This is just an assumption.

I can't say much about NextJ's export variants here either. But I could imagine that these small blocks could be considered as elementary to be integrated in the NextJS project as light as possible. This is also just an assumption.

Maybe something like this helps in the decision
https://spectrum.chat/frontend/opinion/named-exports-vs-default-exports-or-both~32c58d5b-9ba0-46b5-b00d-783f2799ddb4

Very interesting, how Webpack recognizes which parts are unused and can be shaken off, and how this depends on the integration behavior of ES6 and CommonJs export variants.

https://engineeringjobs4u.co.uk/improving-site-performance-with-tree-shaking-coursera-engineering

[aralroca]

@gurkerl83 as a conclusion, we can say:

• Add again the type Translate
• Check whether to put back the named exports

To verify the tree-shaking I will test the bundle analyzer with both types of import. Then I value whether to add them again or not.

[gurkerl83]

Thats great! For the Translate type maybe you can reduce those three t functions to just one, with some arguments optional. If not maybe typescript unions and Intersection types help. Thx!

[vimutti77]

I don't know why. But vscode always suggest to import

import useTranslation from 'next-translate/lib/esm/useTranslation'

instead of

import useTranslation from 'next-translate/useTranslation'

[aralroca]

@vimutti77 I saw it recently... I don't know either... The correct one is as you say:

import useTranslation from 'next-translate/useTranslation'

Because it will load ESM or CJS depending on node / browser.

If someone knows how to tell VSCode what "import" is the correct one, please tell here! I'm going to do some research, thanks to report it.

[aralroca]

Asked in stackoverflow: https://stackoverflow.com/questions/65145253/how-to-tell-vscode-which-suggested-import-is-correct-for-library-creators

I hope someone will enlighten me. I will still continue my research.

[aralroca]

@gurkerl83 I did a prerelease 1.0.0-canary.6 re-adding the Translate type (improved a little bit than 0.20.x). Would you confirm that it works well for you? Thank you! 🙏

[gurkerl83]

Hi thanks for adding this. I use the option to return objects quite frequently, same as the standard variant. Is it possible to change the Generic Typ a bit to support this more properly. I tested the following.

export interface Translate {
  <T = string>(
    i18nKey: string | TemplateStringsArray,
    query: TranslationQuery | null | undefined,
    options: { returnObjects?: boolean; fallback?: string | string[] }
  ): T;
  (
    i18nKey: string | TemplateStringsArray,
    query: TranslationQuery | null | undefined
  ): string;
  (i18nKey: string | TemplateStringsArray): string;
}

A little explanation is necessary for the first option. For the first option I added a generic type to specify in case it is applied the return type of the object. A consumer of t can continue without applying any cast operation like here

 const steps = t<Array<Content>>(
    'pages/ai/index:steps',
    {},
    {
      returnObjects: true
    }
  );

My question is for the second and third option. Are there any circumstances that those functions can return a value other than string. I that case I understand the purpose of <R = string> () : R like

t<Null>("translationKey")

otherwise i would remove the generic type arguments for the second and third option like illustrated at the top of the comment.

this is the version of the canary release 6
export interface Translate {
  (
    i18nKey: string | TemplateStringsArray,
    query: TranslationQuery | null | undefined,
    options: { returnObjects?: boolean; fallback?: string | string[] }
  ): string | object
  <R = string>(
    i18nKey: string | TemplateStringsArray,
    query: TranslationQuery | null | undefined
  ): R
  <R = string>(i18nKey: string | TemplateStringsArray): R
}

Does this makes sense, Thx!

[aralroca]

@gurkerl83 feel free to PR! to be honest, I am not very familiar with TypeScript yet and I am sure it can be better.

[gurkerl83]

I made a merge request. Its base is the canary branch. Please see the commit message for more detail.

[aralroca]

About the named exports, after adding this to the index.ts file:

export { default as DynamicNamespaces } from './DynamicNamespaces'
export { default as I18nProvider } from './I18nProvider'
export { default as Trans } from './Trans'
export { default as appWithI18n } from './appWithI18n'
export { default as loadNamespaces } from './loadNamespaces'
export { default as useTranslation } from './useTranslation'
export { default as withTranslation } from './withTranslation'

I can see that webpack is not doing well the treeshaking... It's importing unused packages...

With named exports

With separate packages

😔

[gurkerl83]

Are you using the package @next/bundle-analyzer in on of the demos, or the standard bundle analyzer on the library?

[aralroca]

I added on the example #379

[gurkerl83]

This needs some more investigation. From a library point of view, I would argue only library relevant files and dependencies have to be taken into account. Applying the analyser at a running application, with other things involved such as mdx loader might corrupt the result and makes interpretation harder. We might consider to make the build process run through rollup of a similar bundler.

[aralroca]

@vimutti77 Finally, I have seen that the auto-import is related to where the TypeScript types are. Moving the types to the root solves it.

Instead of:

{
  "internal": true,
  "main": "../lib/cjs/useTranslation.js",
  "module": "../lib/esm/useTranslation.js",
  "types": "../lib/esm/useTranslation.d.ts"
}

Doing this (After moving the types to the root):

{
  "internal": true,
  "main": "../lib/cjs/useTranslation.js",
  "module": "../lib/esm/useTranslation.js",
  "types": "../useTranslation.d.ts"
}

Solve the problem. Solved package: https://www.runpkg.com/[email protected]/useTranslation/package.json

---

@vimutti77 @gurkerl83 I did a prerelease 1.0.0-canary.7 with the fixed types + fixed auto-import

[vimutti77]

@aralroca That is great!

[aralroca]

@gurkerl83 About the named exports; it is difficult that every export coexists with others. There are exports that have a require('fs') (or other node things) and although webpack does tree-shaking, it first needs to resolve these dependencies, and on the client-side, it can't and is throwing errors about this.

I think for now it is better that they stay as individual packages. Much simpler and this way we avoid conflicts.

[gurkerl83]

I know what you mean, I changed quite some things on my local branch to get it work. The fs problem is best know, and easy to solve. Whats important here is that you have to differentiate the build phase either its browser or server. Each build consits of both. If it is on browser there is no fs.

const getFallback = (isServer) => {
  if (!isServer) {
    return {
      fs: false,
      process: false,
    }
  }
}

Most important when webpack 5 gets used an the project is using it based on the yarn resolution used, node pollifills such as process are not provided anymore. The list goes on, and on.

What made the biggest headache for me was the use of imports in the template sections used, i didn`t realised that in the first place and it was to late to quit e.g.

export function getDefaultAppJs(hasLoadLocaleFrom) {
  return `
  import i18nConfig from '@next-translate-root/i18n'
  import { appWithI18n } from 'next-translate' => **here !!!**

  function MyApp({ Component, pageProps }) {
    return <Component {...pageProps} />
  }

  export default appWithI18n(MyApp, {
    ...i18nConfig,
    skipInitialProps: true,
    isLoader: true,
    ${overwriteLoadLocales(hasLoadLocaleFrom)}
  })
`
}

After a few hours the build was working with named exports, and a rollup build underneath. I clean the stuff I have up and provide a feature, not to merge but for reviewing the approach, because it also splits the loader in a separat build.

I think there is much improvement possible but one step at a time. I`m totally fine with the current export mechanism. Thx!

[aralroca]

Ok this sounds really good 🙏

[aralroca]

@gurkerl83 Maybe we can move this to version 1.1 so that it doesn't block 1.0, what do you think? ☺️

[gurkerl83]

@aralroca Perfect, I currently have to catch up on a few things in other projects and won't be able to clean up and provide my changes until next week.