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

./i18n.js Critical dependency: the request of a dependency is an expression #851

Closed
timheerwagen opened this issue May 3, 2022 · 49 comments
Assignees
Labels
Needs investigation Requires time to do some research
Milestone

Comments

@timheerwagen
Copy link

Today after doing some package updates e.g. next@12.1.6 next-translate crashes while trying to load the locale.
Im importing my locales from a package inside my monorepo with:

./i18n.js

loadLocaleFrom: (locale, namespace) =>
    import(`locales/${locale}/${namespace}`).then((m) => m.default),

That worked for me for like 2 months now.
But now i get following error:

./i18n.js
Critical dependency: the request of a dependency is an expression

After a while of trial and error i got my code working with:

  loadLocaleFrom: (locale, namespace) =>
    require(`locales/${locale}/${namespace}`),

Is the error here with me or is there a package error?

@aralroca
Copy link
Owner

aralroca commented May 3, 2022

If you delete loadLocaleFrom, does it work? By default underneath it makes an identical dynamic import. Looks like an error of the new nextcompiler... Something to investigate. Thanks to report it

@aralroca aralroca added the Needs investigation Requires time to do some research label May 3, 2022
@rodrigodmpa
Copy link

Today after doing some package updates e.g. next@12.1.6 next-translate crashes while trying to load the locale. Im importing my locales from a package inside my monorepo with:

./i18n.js

loadLocaleFrom: (locale, namespace) =>
    import(`locales/${locale}/${namespace}`).then((m) => m.default),

That worked for me for like 2 months now. But now i get following error:

./i18n.js
Critical dependency: the request of a dependency is an expression

After a while of trial and error i got my code working with:

  loadLocaleFrom: (locale, namespace) =>
    require(`locales/${locale}/${namespace}`),

Is the error here with me or is there a package error?

Same thing here, your solution helped me, thanks.

@timheerwagen
Copy link
Author

If you delete loadLocaleFrom, does it work? By default underneath it makes an identical dynamic import. Looks like an error of the new nextcompiler... Something to investigate. Thanks to report it

If i delete loadLocaleFrom i cant load my locales because theyre not in the default folder.
Its a package inside my monorepo.

Structure:

monorepo
 |-apps
   |-project
      |-i18n.js
 |-packages
   |-locales

@derkoe
Copy link

derkoe commented May 10, 2022

We have the same problem and also fixed it with require (like #851 (comment)).

We also had to catch the error on the require because not all of the files exist (we implement a fallback logic for country-specific locales like "es-CL" and then fallback to English):

  const englishTexts = require('./locales/en/myapp.json');
  ...
  loadLocaleFrom: (locale, ns) => {
    let countrySpecific = {},
      languageOnly = {};
    try {
      countrySpecific = require(`locales/${locale.replace('-', '_')}/${ns}.json`);
    } catch (error) {}
    try {
      languageOnly = require(`./locales/${locale.substring(0, 2)}/${ns}.json`);
    } catch (error) {}
    return _.merge({}, englishTexts, languageOnly, countrySpecific);
  },

@fabien
Copy link

fabien commented May 12, 2022

I'm having the same issue after a redeploy (no code changes, just content publishing) on Vercel. Apparently they made Node v.16 the default since May 9th 2022 - but despite setting v.14 (as engine as well as in settings) it appears not to be the culprit as I initially thought.

Unfortunately, the workarounds shown above don't work for me, since I also depend on tsconfig.json path aliases an their dynamic resolving nature:

loadLocaleFrom: (locale, ns) => {
  return import(`@app/translations/${lang}/${ns}`).then(m => m.default);
}

I tried to circumvent this using tsconfig-paths but this relies on fs eventually, which obviously doesn't work.

Does anyone have more details when and how this 'new' behaviour is triggered? Any way to opt-out if this?

EDIT: the workaround does work, even with require and the path aliases!

@derkoe
Copy link

derkoe commented Jun 3, 2022

The problem with using require is that all locales will be included in the _app bundle. This makes the first load quite big if you have many languages (as we do). See also #741

@derkoe
Copy link

derkoe commented Jun 3, 2022

Just looked at the code and saw that the default loader is passed as a string: https://github.com/vinissimus/next-translate/blob/49f580c6d292712a0720a104de3b487e7c11d4ae/src/plugin/utils.ts#L6-L7

I've tried to use a string now in our repo and it works :-)
i18n.js:

module.exports = {
  loadLocaleFrom: '(lang, ns) =>  import(`./locales/${lang}/${ns}.json`).catch(e => console.error(e)).then((m) => m.default)',
}

The only issue with that is that you cannot load libs in the string (like lodash).

Update: no it does not - because it checks if the loadLocaleFrom is a function:
https://github.com/vinissimus/next-translate/blob/49f580c6d292712a0720a104de3b487e7c11d4ae/src/plugin/index.ts#L83

@derkoe
Copy link

derkoe commented Jun 3, 2022

When disabling the loader the imports still work - see #741 (comment) for an example.

@aralroca
Copy link
Owner

aralroca commented Jul 4, 2022

Yep, I don't recommend require because you are adding more kb to the initial chunk, this was the grace of the dynamic import, to import only what you need.

From what I have seen it fails in the new version of Next.js when the file is from node (it has module.exports). For example, if you put any dynamic import inside any page the same thing happens:

_app.js

import { loadSomethingWithDynamicImport } from '../example'

export default function App(){
  // ...
}

App.getInitialProps = async (context) => {
    const res = await loadSomethingWithDynamicImport('bla')
}

Is working fine whenexample.js is like:

export function loadSomethingWithDynamicImport(param) {
  return import(`something/${param}`).then(r => r.default)
}

However it crash with the same error if example.js is like:

function loadSomethingWithDynamicImport(param) {
  return import(`something/${param}`).then(r => r.default)
}

module.exports = { loadSomethingWithDynamicImport }

Does anyone have any idea how to solve it without having to use require? Thanks!

@aralroca
Copy link
Owner

aralroca commented Jul 4, 2022

With the default loadLocaleFrom is working fine, the problem in Next.js 12 is where loadLocaleFrom is redefined inside i18n.js because has a module.exports

@aralroca aralroca self-assigned this Jul 4, 2022
@aralroca
Copy link
Owner

aralroca commented Jul 4, 2022

Of course, it is not the right solution, but until we find the best way to fix it (all suggestions are welcome), you can do a workaround and not use the loadLocaleFrom inside i18n.js and overwrite the default:

const workaround = require('next-translate/lib/cjs/plugin/utils.js')

// As a workaround you can change the string of the defaultLoader, this is working fine
workaround.defaultLoader = '(l, n) => import(`@next-translate-root/src/locales/${l}/${n}.json`).then(m => m.default)';

module.exports = {
  ...config,
  // loadLocaleFrom: (lang, ns) =>
  //   import(`./src/locales/${lang}/${ns}.json`).then((m) => m.default),
}

Any suggestions from anyone on how to fix it internally in the library? Thanks!

@andresz1
Copy link

The workaround doesn't work for me 😢 any suggestion ? willing to help

@mattiaz9
Copy link

The workaround doesn't work for me 😢 any suggestion ? willing to help

I just use this option:

...
loadLocaleFrom: (lang, namespace) => require(`./lang/${lang}/${namespace}.json`),
...

but I'm having issues with the latest version 1.5, so I had to downgrade to 1.4

@aralroca
Copy link
Owner

aralroca commented Jul 10, 2022

@mattiaz9 What errors do you have in 1.5? Anyway, as I said here and a workaround to solve it here, better not to use require because it will increase your bundle size.

@mattiaz9
Copy link

@aralroca I get this error: TypeError: conf.loadLocaleFrom(...).catch is not a function

@mattiaz9
Copy link

Actually.... I just tested the workaround and it seems to work fine.

This is the correct code:

const workaround = require("next-translate/lib/cjs/plugin/utils.js")

workaround.defaultLoader = "(l, n) => import(`lang/${l}/${n}.json`).then(m => m.default)"

module.exports = {
  // config without 'loadLocaleFrom'
}

@aralroca
Copy link
Owner

Cool. The ideal is to solve it at the library level without this workaround. I am not very clear on how. It is like a bug (or normal behavior on purpose) of SWC, that crashes the dynamic import only when a file with module.exports has it.

@marcusnunes
Copy link

Same problem, I had to downgrade to 1.3.0.

loadLocaleFrom: (locale, namespace) =>
  require(`./src/locales/${locale}/${namespace}`),

@aralroca
Copy link
Owner

aralroca commented Aug 4, 2022

Same problem, I had to downgrade to 1.3.0.

loadLocaleFrom: (locale, namespace) =>
  require(`./src/locales/${locale}/${namespace}`),

Is this workaround not working for you? #851 (comment) . Better not to use require in order not to increase the bundle size.

@itxtoledo
Copy link

same issue here with next 12.2.5

@weee-shinan
Copy link

I solved this problem by manually adding the .babelrc file, and using next/babel to process it will not report an error, you can try it @mattiaz9 @fabien @derkoe @andresz1 @marcusnunes

@aralroca
Copy link
Owner

aralroca commented Sep 5, 2022

I solved this problem by manually adding the .babelrc file, and using next/babel to process it will not report an error, you can try it @mattiaz9 @fabien @derkoe @andresz1 @marcusnunes

It's because the bug is added by SWC, by adding the babel you stop using the new Next.js rust compiler. I recommend using the workaround until we find the best solution: #851 (comment)

@marcusnunes
Copy link

I just moved my locales from /src/locales to /locales and it worked here.

loadLocaleFrom does not work in the current version

@aralroca
Copy link
Owner

aralroca commented Sep 5, 2022

loadLocaleFrom does not work in the current version

Yep... The new rust compiler of Next.js (SWC) interprets the loadLocaleFrom from i18n.js as server code (because it have a module.export), and SWC doesn't like dynamic imports in server files.

The same error is reproducible without next-translate, creating a file with a dynamic import and module.export:

function loadSomethingWithDynamicImport(param) {
  return import(`something/${param}`).then(r => r.default)
}

module.exports = { loadSomethingWithDynamicImport }`

To solve the problem from NextTranslate we would have to see how we can solve this of the module.export so that SWC interprets correctly the dynamic import, or look for an alternative of how to pass this loadLocaleFrom function. Any proposal will be very welcome since it is a problem that has to be solved but it is not clear to me how we can solve it for now.

@donthijackthegit
Copy link

Same here, I have to roll back to next@12.1.0 and problem resolved

@donthijackthegit
Copy link

donthijackthegit commented Nov 7, 2022

Upgraded to next@12.3.2, need to return promise

loadLocaleFrom: (lang, ns) => {
        // You can use a dynamic import, fetch, whatever. You should
        // return a Promise with the JSON file.
        // yaml already being transformed by webpack plugin
        const m = require(`./locales/${lang}/${ns}.yml`)
        return Promise.resolve(m)
    }

@urmauur
Copy link

urmauur commented Nov 12, 2022

const m = require(`./locales/${lang}/${ns}.yml`)
        return Promise.resolve(m)

work for me using this 👌🏻

@felipeferrarini
Copy link

Upgraded to next@12.3.2, need to return promise

loadLocaleFrom: (lang, ns) => {
        // You can use a dynamic import, fetch, whatever. You should
        // return a Promise with the JSON file.
        // yaml already being transformed by webpack plugin
        const m = require(`./locales/${lang}/${ns}.yml`)
        return Promise.resolve(m)
    }

I'm using files in json format and worked too 🙌

@aralroca
Copy link
Owner

@felipeferrarini @urmauur @donthijackthegit This issue was introduced after the SWC rust compiler, this compiler doesn't support dynamic import in Node (files with module.export and require).

As a workaround, I recomend this for now

This workaround is avoiding the require, which loads all the namespaces in Webpack instead the only one that you need

@aralroca
Copy link
Owner

aralroca commented Jan 24, 2023

Hello everyone 😊

We are considering making an important change to our library's configuration, where the loadLocaleFrom method currently returns a promise, but in the future it could also return a string.

loadLocaleFrom: (locale, namespace) => `src/translations/${namespace}_${locale}.json`,

With this change, we would be able to use our own Webpack plugin to calculate all available languages and namespaces and place them in separate chunks, thereby improving the performance of the application. This plugin is already implemented in our library and has other functionalities in addition to this. Nevertheless, we will retain the option of returning a promise, to allow users to make fetch or implement their own loading logic.

However, before proceeding with this change, we would like to hear your opinion. How do you think this modification would affect your use of the library? Are there any specific use cases in which this modification could cause problems? Any other suggestions or comments on the matter?

We appreciate your time and welcome any feedback you may provide. ❤️

@lukevella
Copy link

Sounds reasonable. Have you considered going a step further and just offer an option to change the root path?

{
   "localeRoot":  "src/translations"
   // equivalent to  loadLocaleFrom: (l, n) => `src/translations/${l}/${n}.json`
}

The library already expects this folder structure out of the box and seems a bit overkill to use loadLocaleFrom when all you need is to specify a different root. loadLocaleFrom can still exist of course for anyone that needs a different structure.

@aralroca
Copy link
Owner

@lukevella thanks for the comment, I like the idea of keeping things simple, and I had thought about it, although I don't like having two configurations for something similar. 🤔

I've seen many cases of people changing the structure of namespaces, either because they support many types of English (UK, US) but many translations are shared and it doesn't make sense to have them duplicated, or namespaces in other formats that are not json, etc... All of these cases should also benefit from having the namespace chunks separated, and that's why I propose the change above.

This issue was introduced because the SWC does not support dynamic import here now and by replacing it with require, it puts all the namespaces in the same chunk, making each page load all the translations instead of just the ones necessary for the page.

With this change, everything is solved; if you want to change the folder, the structure, load it from external resources with a fetch, etc.

@lukevella
Copy link

lukevella commented Jan 24, 2023

Sure, it would just be great if there wasn't a need to switch from .json to .js for this from a DX perspective. Maybe this can all be handled by having loadLocaleFrom also accept string | () => string.

{
   loadLocaleFrom: "src/translations",
   // equivalent to  loadLocaleFrom: (l, n) => `src/translations/${l}/${n}.json`
}

Either way seems like a step in the right direction as not needing to use dynamic imports in the config would be great for DX.

@aralroca
Copy link
Owner

aralroca commented Jan 24, 2023

I'll keep that in mind, it's a good idea, thank you @lukevella . Maybe we can extend its use to:

string | () => string | () => Promise<json>

@aralroca
Copy link
Owner

aralroca commented Jan 25, 2023

As a workaround, until the new functionality is implemented, if someone wishes to use the require method but have all chunks separated, they can utilize the webpack property provided below to resolve the issue. This sets up a webpack configuration that splits the chunks based on the namespace and locale, so that all chunks are separated and can be easily identified.

const namespaces = ['common', 'dynamic', 'home', 'more-examples', 'error'];
const locales = ['en', 'ca', 'es'];

module.exports = nextTranslate({
  webpack: (config) => {
    namespaces.forEach((namespace) => {
      locales.forEach((locale) => {
        config.optimization.splitChunks = {
          ...config.optimization.splitChunks,
          cacheGroups: {
            ...(config.optimization.splitChunks.cacheGroups || {}),
            [`${namespace}_${locale}`]: {
              test: new RegExp(`src/locales/${locale}/${namespace}.*$`),
              name: `${namespace}_${locale}`,
              chunks: 'all',
              enforce: true,
            },
          }
        }
      })
    })

    return config
  },
})

Then is not a problem using require in i18n.js file:

{
  // ... Rest of config
  loadLocaleFrom: async (locale, namespace) =>
    require(`./src/locales/${locale}/${namespace}.json`),
}

@geonwoomun
Copy link

@aralroca
This way it is divided into chunks, but loaded all at once on first load. 🥲

@aralroca
Copy link
Owner

aralroca commented Feb 9, 2023

@aralroca This way it is divided into chunks, but loaded all at once on first load. 🥲

wow...! Thanks to detect it @geonwoomun! 😅 So we need to find a solution for this... Do you have some proposals? for now that we are planing to release 2.0.0 we can add a breaking change to fixloadLocaleFrom if is really necessary...

@geonwoomun
Copy link

geonwoomun commented Feb 10, 2023

@aralroca
I used derkoe's method shown here.

Let me show you part of my code...

_app.tsx

export default appWithI18n(App, {
  ...i18nConfig,
  loadLocaleFrom: (lang, ns) => {
    const langCode = lang ? supportLanguagesMap[lang] : fallbackLanguage;
    return import(`../../public/locales/${location}/${langCode}/${ns}.json`);
  },
});

i18n.js

const i18nConfig = {
  locales: ~~~,
  defaultLocale: 'en',
  pages: {
    '*': ['common'],
  },
  loader: false, // this is required
  interpolation: {
    prefix: '{',
    suffix: '}',
  },
  defaultNS: 'common',
};

By doing this, I confirmed that it works as well as when using babel, is divided into chunks, and does not load all at first.

@aralroca
Copy link
Owner

@geonwoomun the problem with dynamic imports in SWC is in files with module.exports =, like i18n.js file. Adding in _app.js works fine yes, however we want a solution inside i18n.js. This file is read inside the next.config.js (using our webpack plugin).

@kdy1
Copy link

kdy1 commented Feb 10, 2023

I don't think it's a bug of swc. I'm not sure if I got your sentence, though.

@geonwoomun
Copy link

@aralroca
I asked swc maintainer kdy and he answered above.

@aralroca
Copy link
Owner

@kdy1 A summary of this issue:

In previous versions of Next.js prior to version 12, dynamic import in the configuration worked correctly. It allowed next-translate users to choose where they wanted to have the translation files and only download the chunk that corresponds to the current page. For example, if the page was in English, only the English translations would be loaded, not all languages.

This was possible because the configuration was read within the Webpack plugin that we have, which is executed within the next.config.js file, and during the parsing of the files, the import was added in the loader of each page (getStaticProps, etc.) so that users wouldn't have to do it manually on each page.

However, this stopped working correctly in versions equal to or greater than Next.js 12, and the following error occurred using dynamic import:

Critical dependency: the request of a dependency is an expression

It worked with Babel because it was supported with this plugin, which suggests that it is not supported with SWC.

Starting from version 12, next-translate users have to replace the dynamic import with require.

-return import(`./locales/${langCode}/${ns}.json`)
+return require(`./locales/${langCode}/${ns}.json`)

It loads the translations and the application works fine... but unlike before, it loads all the translations instead of just the one that the page needs.

Since then, we have been looking for a way to solve this at the configuration level, to find an alternative to the dynamic import and require, and to resolve this in an easy way for users...

Of course, the most practical solution would be for dynamic import to continue working. After researching, dynamic import only works with files with ECMAScript 6 Modules, but with files such as CommonJS it does not. Moving this configuration logic directly to the page (ECMAScript 6 modules) does work for dynamic import.

@kdy1
Copy link

kdy1 commented Feb 10, 2023

Dynamic import is a standard and it's supported by swc

@kdy1
Copy link

kdy1 commented Feb 10, 2023

AFAIK there's auto_cjs pass which changes module mode to commonjs based on the content. Can you file an issue?

@kdy1
Copy link

kdy1 commented Feb 10, 2023

Nevermind, I'll use vercel/next.js#40231

@aralroca
Copy link
Owner

@kdy1 thanks for this PR vercel/next.js#45836 ! Great news 🎉🥳

This issue is already fixed in Next.js 13.1.7-canary.11 without any change on Next-translate configuration, you can still use dynamic import inside 👍 😊

@laymonage
Copy link

For anyone else upgrading to next-translate@2 but still stuck on Next.js 12 (e.g. because Preact is not yet supported by Next.js 13), you need to make some changes to the workaround mentioned in #851 (comment):

- const workaround = require('next-translate/lib/cjs/plugin/utils.js')
+ const workaround = require('next-translate-plugin/lib/cjs/utils.js');

And the workaround should not be placed in i18n.js, because the plugin's utils.js now imports TypeScript. If you put the workaround in i18n.js, TypeScript will be included in your application bundle, massively increasing its size.

I placed it directly in next.config.js and it seems to work fine, though I did have to extract some of the data needed for the defaultLoader workaround string into a separate file.

Glad to hear it's been fixed in Next.js, though! Thanks to everyone involved 😄

@mattvb91
Copy link

so ive finally managed to get my __namespace loading with the solutions from above, but the only reason I jumped through the hoops trying to get this working is because I need to manually set the language through getInitialProps, which I now cant do anymore because it uses the locale coming from appWithI18n. Does anyone have any idea how to manually change the lang from inside a global _app getInitialProps?

@Macumba45
Copy link

Hello guys,

I cant find a solution...

I have nextJs 13 and next-translate-plugin.

In local works perfect but when i upload to Vercel´s server, the translations does not works...

Do you know what could be? I am stucked more than 2 days and i am lost already....

Thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs investigation Requires time to do some research
Projects
None yet
Development

No branches or pull requests