Closed aralroca closed 3 years ago
If someone wants to experiment, I did a first experimental version under 1.0.0-experimental.1
prerelease.
Things are still missing, so several things may still fail, but I prefer you to experiment it, and if you find something, report it here (please don't do PR yet to that branch).
Note: instead of localesPath
, now is loadLocaleFrom
(already in the doc) for all the cases: build step, custom server, etc. By default is getting the files from locales
folder as before.
When trying to load a normal page I get the following error in the console:
When trying to load a page with getInitialProps I get:
error - ./node_modules/next-translate/index.js:1:833
Module not found: Can't resolve 'fs'
null
TypeError: Cannot read property 'prototype' of undefined
at loadGetInitialProps (/Users/bjoern/projects/inventhora/page-builder/node_modules/next/dist/next-server/lib/utils.js:3:690)
at appGetInitialProps (/Users/bjoern/projects/inventhora/page-builder/node_modules/next/dist/pages/_app.js:4:106)
at /Users/bjoern/projects/inventhora/page-builder/node_modules/next-translate/appWithI18n.js:1:2454
at tryCatch (/Users/bjoern/projects/inventhora/page-builder/node_modules/regenerator-runtime/runtime.js:63:40)
at Generator.invoke [as _invoke] (/Users/bjoern/projects/inventhora/page-builder/node_modules/regenerator-runtime/runtime.js:293:22)
at Generator.next (/Users/bjoern/projects/inventhora/page-builder/node_modules/regenerator-runtime/runtime.js:118:21)
at asyncGeneratorStep (/Users/bjoern/projects/inventhora/page-builder/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
at _next (/Users/bjoern/projects/inventhora/page-builder/node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
at /Users/bjoern/projects/inventhora/page-builder/node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
at new Promise (<anonymous>)
Could not find files for /project in .next/build-manifest.json
Not sure if I configured it right, my config:
//i18n.js
module.exports = {
locales: ['en', 'es', 'de', 'pt'],
defaultLocale: 'en',
loadLocaleFrom: (lang, ns) =>
import(`./locales/${lang}/${ns}.json`).then((m) => m.default),
pages: {
'*': ['common'],
},
}
//next.config.js
require('dotenv').config()
const withI18n = require('next-translate')
module.exports = withI18n({
env: {
BASE_URL: process.env.BASE_URL,
},
})
and that's it, right?
@BjoernRave Try with 1.0.0-experimental.2 that now it has some corrections with respect to the experimental.1
. If it doesn't work either, you would paste here your code of your _app.js
file? π Thank you!
@aralroca I am getting a Module not found: Error: Can't resolve 'fs
error when using 1.0.0-experimental.2
. I am using TypeScript, NextJS 10, React 16, a very simple babel file... what further information would you need to help me pinpoint this?
I usedfs
inside the plugin to read the configuration file (i18n.json
). I don't have any problem with it, but maybe it could be related to this https://github.com/vercel/next.js/issues/7755#issuecomment-508633125 ?
If adding this works:
module.exports = {
webpack: (config, { isServer }) => {
// Fixes npm packages that depend on `fs` module
if (!isServer) {
config.node = {
fs: 'empty'
}
}
return config
}
}
I will include it inside the same plugin to make it work. Can you confirm it? π and thank you very much for the feedback!
if you can provide a simple repo reproducing the error, it would be very helpful π
That fixed it indeed! Also, check out this comment: https://github.com/vercel/next.js/issues/7755#issuecomment-624544303 That might be the solution?
EDIT
Yep, that comment is right. If I simply include any fs
call, such as
fs.stat("/a.tsx", () => {});
in my getStaticProps
method, then NextJS' babel plugin removes that import from the browser plugin and that error disappears
However, now I get this error:
Error was not caught ReferenceError: exports is not defined
at Module.eval (webpack-internal:///./node_modules/next/dist/pages/_app.js:8)
at eval (webpack-internal:///./node_modules/next/dist/pages/_app.js:108)
You can reproduce my setup here: https://github.com/jan-wilhelm/next-translate-error-reproduction
Update: I have found that the error appears when using react@16.14.0, but disappears on react@17.0.1
Question - Is there any way at all for next-translate
not to use getServerSideProps
? I am trying to set up a NextJS application where we have a route like products/[productId]
. I do not know all possible productIds at build time, as they can constantly change. However, I want that ProductID-Page to be statically prerendered and to use next-translate. NextJS currently forces me to export all possible getStaticPaths
.
Apparently, next-translate currently circumvents this by using getServerSideProps
which disables automatic static optimisation. However, the loading screen for each productID is always the same and content is only fetched on the client side, so theoretically NextJS should just be able to pre-generate the loading screen for all any productID.
I wouldn't normally need any of getStaticProps
, getStaticPaths
or getServerSideProps
at all on this page, however next-translate currently forces me to use SSR :/
(should this be a new issue?)
@jan-wilhelm next-translate
now uses the existing loaders if the page already has one. If your page has a getStaticProps
, it will use a getStaticProps
to load the translations. The problem is with the default values, when there is no existing loader. In this case, there are 3 cases:
getStaticProps
to have maximum performance.getServerSideProps
because it is required by Next.js that if you want to provide the getStaticProps you must also write the getStaticPaths, and we have no idea of the paths that will be used, and to avoid putting a "fallback: false", we decided that perhaps, in this case, it was better that the fallback was the getServerSideProps
.getInitialProps
by default to avoid possible conflicts.However, remember that these are default values when the page has no loader. Writing an empty getStaticProps
will cause next-translate
to use getStaticProps
to load the translations.
@jan-wilhelm Do you think it would make more sense for the fallback for dynamic pages ([slug]) to provide a getStaticPaths with fallback: false? I would love that next-translate uses a getStaticProps by default in this case too, but at the same time I don't like that the translations are loaded in CSR without having the page prerendered
yeah I totally get that. What would you suggest to do in this scenario? Are we just gonna have to wait for NextJS to support getStaticProps
without getStaticPaths
? It feels wasteful to me to always render the same loading screen skeleton over and over again, because I do not want to use SSR.
I think personally, for me, I could either go with the fallback: true
approach or make it so that the page is not dynamic (something like products/?id=PRODUCT_ID
so that it can get pre-rendered. I think it is fine for next-translate to keep using getServerSideProps
for this by default.
I asked the same question on a next-i18next GitHub Issue and I'm curious to see what option they will choose.
I would say that the easiest way is to use SSR temporarily for these pages (unless you know all the paths). It is the simplest thing, although it has the cost of optimization, we know that it is temporary. If you make temporary alternative routes I think it will be much worse to change it in the future, since you will have to provide a 301 redirect to the new route for each of your products, and you could have serious SEO problems.
The i18n support of Next.js right now is only routing, but it is also in their plans to develop a method to load the translations. So this would solve your problem.
A little bit the idea to make now the plugin is about the future of i18n inside Next.js. We want that in a future it will be very easy to migrate from one way to another just by upgrading Next.js and next-translate without breaking changes, and gaining then optimization in these pages.
I guess and hope that the development about loading the translations from Next.js team will be declarative at the configuration level. It's clear that writing manually in each page which namespaces you want inside a loader (getStaticProps, getServerSideProps) is a pain, and that's why we give the opportunity to define it this way:
{
"locales": ["en", "de"],
"defaultLocale": "en",
"pages": {
"*": ["common"], // all pages
"/": ["home"],
"/product/[id]": ["product"],
"rgx:^/account": ["account"], // all pages under /account
"rgx:^/admin": ["admin"] // all pages under /admin
}
}
Hello, I have tested the Feature Branch 1.0.0 experiment. The setup of my Next Configuration looks like this.
const { merge } = require('webpack-merge');
const nextTranslate = require("next-translate");
const nextConfig = {
// Called within next-Translate, criteria the property webpack a function
webpack: (config, options) => {
return merge(
config,
process.env.NODE_ENV === 'development
? webpackConfigDev(config, options)
: webpackConfigProd(config, options)
);
},
module.exports = nextTranslate(nextConfig)
From the configuration you can see how to use Webpack. An essential module contained in the feature supplements the Next-Translate Loader in any detectable module rules, which were created by upstream Webpack steps. In my case these are not only the steps defined by NextJs, but also those that are created in the mentioned merge parts depending on whether development or production. Whether a local or outsourced Webpack configuration is used here is not questionable.
The call of dedicated Webpack steps is controlled by next-translate.
So projects that have their own Webpack configuration must be called by the call.
Adaptation 1: In NextJs there is a request to block the FS if !server
webpack(conf) {
const config =
typeof nextConfig.webpack === 'function
? nextConfig.webpack(conf)
: conf
webpack(conf, options) {
const config =
typeof nextConfig.webpack === 'function
? nextConfig.webpack(conf, options)
: conf
Adaptation 2: The Branch 1.0.0-experimental.2 is not compatible to Webpack 5. The existing code creates undefined values in the array if "use". The existing code distinguishes two variants, if array or not. If the variable is not an array it is expected to be an object.
At least one case I could identify, where this expectation does not come true.
Input:
{ test: /\.m?js/, resolve: { fullySpecified: false } },
Output:
{ test: /\.m?js/, resolve: { fullySpecified: false }, use: [] },
const use = Array.isArray(r.use) ? [...r.use, loader] : [r.use, loader]
let use = [];
if(array.isArray(r.use)) {
use = [...r.use, loader];
} else if (typeof r.use === 'object') {
use = [r.use, loader]
} else if ( r !== null && r.use === null) {
use = [{...r}, loader]
}
What needs to be clarified additionally is how to handle loaders which follow an outdated nesting specification, those where the specification differs from module/rules/ (test|use). Maybe not relevant here.
module.loaders were deprecated since Webpack 2 and are now removed in favour of module.rules.
Question to the overall approach. As seen in the last step, all loaders are mutated exactly on the first pass. My question is what exactly should be achieved when this is done across all rules, to mount the next-translate Loader.
Is it not enough to do this exactly for Javascript files?
Maybe it is possible that someone will validate, improve and experiment with the proposed changes via NPM.
Thanks for listening and keep up the cool library!
@gurkerl83 Thank you very much for your feedback! π About passing the webpack configuration all the way through is something I'm going to fix right now! π Thanks! About webpack 5 support is on the TODO list! heh! In the same file above there are some comments about everything that is still missing. I will take into account everything you tell me to continue developing version 1.0, very useful your comments.
Question to the overall approach. As seen in the last step, all loaders are mutated exactly on the first pass. My question is what exactly should be achieved when this is done across all rules, to mount the next-translate Loader.
Is it not enough to do this exactly for Javascript files?
I don't know if I understood the question correctly. You mean that instead of making a .map
and replacing the existing JavaScript loader to include our loader what I should do is add another independent loader inside rules
?
The truth is that I also thought about doing it this way, and maybe we can change the way we load it. Basically, I preferred to use this approach because Next.js already adds which files to include and which to exclude, so instead of taking all the .js
files, it only enters the necessary ones. With the idea that if these changes in the future because Next.js changes its internal structure, with this implementation our loader will not be broken.
But, now you have made me wonder that maybe this is not enough reason. So maybe it makes more sense to add an independent rule
with our loader, making sure that it always runs before Babel. With this perhaps if it would be enough, excluding node_modules
and little more.
@aralroca I saw you made the changes, and provided a new experimental release. Unfortunately it is not working. The reason is that you have initialised the variable use with the loader.
let use = [loader] // use has to be initialised with an empty array => let use = []
For the webpack options please do not use the spread operator here, there is no reason to do that but makes code more complicated to understand. The wepack interface has two arguments, the first one is config (here conf), the second one is options, options is where isServer lives.
webpack(conf, ...rest) {
const config =
typeof nextConfig.webpack === 'function'
? nextConfig.webpack(conf, ...rest)
: conf
just replace it with
webpack(conf, options) {
const config =
typeof nextConfig.webpack === 'function'
? nextConfig.webpack(conf, options)
: conf
Thx!
@gurkerl83 thanks for this amazing feedback, I applied the changes under 1.0.0-experimental.4
π π
Hi @aralroca, with 1.0.0-experimental.10 I get the following compile error when using the Trans
component:
Its return type 'string | ReactNode[]' is not a valid JSX element.
Type 'string' is not assignable to type 'Element'.
82 |
83 | <Form.Text muted>
> 84 | <Trans i18nKey="signup:acceptTermsDescription" components={[<Link href="/terms">
| ^
85 | <a>terms</a>
86 | </Link>]}/>
87 | </Form.Text>
Changing the return type to JSX.Element
or React.ReactElement
solves the compile error, however I don't know what is recommended in this case.
@chrisss404 thank you to report it. I fix it on 1.0.0-experimental.11.
I'm taking advantage to migrate the project to TypeScript, so if you see more type errors comment it here or do PR yourself in the 1.0.0-experimental branch! Thanks a lot!
@aralroca
Using version1.0.0-experimental.13
, I set up my next.config.js
like this, but it doesn't work. Is this still how you apply the plugin?
const nextTranslate = require( 'next-translate')
module.exports = nextTranslate({})
@andrehsu are you getting this error? For this error you should update React to 17 (meanwhile I can't find a way to fix it). If is another error, I would appreciate it very much if you would report what you get. Thank you very much! π
@aralroca
I did not get that error, I was already on react 17.0.1
.
My IDE inspection gave a warning on nextTranslate
saying Method expression is not of Function type
, don't know if that's relevant or not.
@andrehsu are you using Webstorm? I see this: https://intellij-support.jetbrains.com/hc/en-us/community/posts/206346419--Method-expression-is-not-of-function-type-error-on-Typescript-constructor-call (I dunno if is related)
@aralroca
Yes I am, but the issue exists independent of WebStorm.
However, now I get this error:
Error was not caught ReferenceError: exports is not defined at Module.eval (webpack-internal:///./node_modules/next/dist/pages/_app.js:8) at eval (webpack-internal:///./node_modules/next/dist/pages/_app.js:108)
You can reproduce my setup here: https://github.com/jan-wilhelm/next-translate-error-reproduction
@jan-wilhelm it should work with 1.0.0-experimental.15 π
I've already fixed it, but if I'm honest, I haven't understood why... Let's see if someone can explain it to me!
I was debugging and saw that it was something in the code that caused TypeScript to compile badly. So, I was uncommenting line by line, until I saw what was wrong with the line:
import App from 'next/app'
// Inside getInitialProps wrapper...
App.getInitialProps() // THIS LINE
It seems that when commenting App.getInitialProps()
then TypeScript it compiles well. Fortunately, we have been able to dispense with this line of code and now with prerelease 1.0.0-experimental.15 it seems to work well with React 16.
I don't really understand why this could be happening, could it be because we have Next.js as peerDependency? I hope someone can clarify for me why this was happening π
The migration guide to 1.0.0 is still available on https://github.com/vinissimus/next-translate/blob/1.0.0-experimental/docs/migration-guide-1.0.0.md (replacing the version to the latest experimental prerelease it should work).
Everything is already quite stabilized, in a short time we will surely go from the experimental version to the canary, since it looks like we will finally evolve around here. And I hope that the final version 1.0.0 will be available in December.
It would help a lot if you could try the latest prerelease and report possible problems; things in the documentation, issues... π
I tested 1.0.0-experimental.16"
. Things were running smooth, only switching language was not really working. This is my logic:
const changeLanguage = (lng: string) => {
if (router.asPath === '/') {
router.push('/', '/', { locale: lng })
} else {
const routes = router.asPath.split('/')
if (allLanguages.includes(routes[1])) {
routes.splice(1, 1)
}
const route = routes.join('/') ? routes.join('/') : '/'
router.push(route, route, { locale: lng })
}
}
the url path get's changed and the locale
value on useRouer
as well, but the translations stay the same.
my i18n.js:
module.exports = {
locales: ['en', 'es', 'pt'],
defaultLocale: 'en',
loadLocaleFrom: (lang, ns) =>
import(`./locales/${lang}/${ns}.json`).then((m) => m.default),
pages: {
'*': ['common'],
'/': ['home'],
'/signup': ['signup', 'forms'],
'/socialsignup': ['signup', 'forms'],
'/checkout': ['signup', 'forms'],
'/privacy': ['privacy'],
'/cookies': ['cookies'],
'/terms': ['terms'],
'rgx:/admin$': ['admin'],
'/features': ['features'],
},
}
@BjoernRave I have copied your changeLanguage
function in the examples of the experimental branch and tried several combinations and it seems to work in all cases. Perhaps it would help me if you could provide a reproducible example.
By the way, the last experimental one, for now, is the 1.0.0-experimental.18
, with this one it happens too?
Could it happen on a particular page that you have getInitialProps
on this page? There is this known issue, which has been going on since version 0.19:
In +0.19 we are displaying this warning:
I see that now this warning is lost, I'll add it again.
Do you think it could be that?
Thank you!
ah yes, getInitialProps
might be the problem. When switching the language on a page without it, it works fine
so version 1.0
will not support the usage of getInitialProps
?
@BjoernRave the idea is that it works the same as next-translate >= 0.19 where we moved all the routing part of i18n to the way that now the Next.js v10 core has.
In Next.js version 10 they support i18n within:
But the support for i18n within getInitialProps on individual pages is missing. I understand that this is a problem, so I opened the issue in the Next.js repo. However, it is something that until it is solved in the Next.js core will not have a solution for now.
And understanding that this issue exists since version 0.19 and comes from Next.js core, I guess that version 1.0.0 will leave without this. When there is a solution will only need to upgrade the version of Next.js.
Perhaps I will try to contribute in Next.js to try to add this, I suppose that other i18n libraries will also miss it.
In fact, we have it implemented. If getInitialProps from Next.js sends the locale in its context, then it will work without another release.
got it, thanks for the clarification and your dedication to make everything work as smooth as possible :)
Closing this. Already implemented on 1.0.0-canary.1. The 1.0 will be released approximately 1-2 weeks. π
Now we have changed the way we do the "build". Before it made much more sense, because there was no i18n routing support in Next.js, so what we were doing was creating all possible static pages, working in a parallel directory like
pages_
.This now after 0.19 has evolved in such a way that we only need the build to load the namespaces, so it's time to think of a better alternative.
I have been doing tests in the 1.0.0-experimental branch and I see it very feasible to move this logic to a Next.js plugin, moving the builder part to a webpack loader.
This is going to fix:
pages
directory than Next.js. No need of workarounds.locales
anddefaultLocales
onnext.config.js
, this will be injected by the plugin. So you only will have ai18n.js
ori18n.json
root file.The only change (after renaming
pages_
topages
) will be:Instead of this on package.json:
Use in
next.config.js
file:The idea is to keep the same
i18n.json
/i18n.js
configuration file.