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

Add translation files with plugin config #810

Closed
Draykee opened this issue Apr 6, 2021 · 14 comments
Closed

Add translation files with plugin config #810

Draykee opened this issue Apr 6, 2021 · 14 comments

Comments

@Draykee
Copy link
Contributor

Draykee commented Apr 6, 2021

Is your feature request related to a problem? Please describe.
When I create a new error message in my plugin, I want them to be translated like the other vendure errors.

This might not be necessary for the frontend but it might be easier for the frontend developer to always keep the same error format.

Describe the solution you'd like
A possiblity to add message files ( like the ones in core/src/i18n/message ) via PluginConfig.
All messages should be merged together on bootstrap.

Additional context
Translations can already be merged in the admin ui - so this might be reusable.

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

@michaelbromley Are there special guidelines I should try to follow?
Like should I add a new config option to VendurePluginMetadata to import i18n files? Maybe it's also a thing to add it during runtime in the plugin constructor via the I18nService

I think the last idea would be the easiest one. I could add a concat function that uses i18next.addResourceBundle

@michaelbromley
Copy link
Member

Yes, I prefer the second solution, because I want to keep the API of PluginMetadata as limited as possible.

So the idea would be that in your plugin you inject I18nService and then call some new method, e.g. addTranslations() which could take a path to a json file, or an array of such.

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

Just as a reminder: i18next-node-fs-backend seems to be deprecated.

@michaelbromley
Copy link
Member

oh, I didn't know that! Looks like it can be replaced with https://github.com/i18next/i18next-fs-backend, they say

It's based on the deprecated i18next-node-fs-backend and can mostly be used as a drop-in replacement.

I wonder what "mostly" means here. Are you interested in attempting to try?

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

Yeah I can try that. But right now im struggeling finding the correct namespace where to place the new resource bundle at.

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

I have to import the I18nService from a wierd place:
import { I18nService } from '../../../dist/i18n/i18n.service';

Isn't it exported in '@vendure/core'?

This is the error I get when I try to import from Vendure core in my e2e test plugin

@vendure/core:       111 |         try {
@vendure/core:       112 |             DefaultLogger.hideNestBoostrapLogs();
@vendure/core:     > 113 |             const app = await NestFactory.create(appModule.AppModule, {
@vendure/core:           |                         ^
@vendure/core:       114 |                 cors: config.apiOptions.cors,
@vendure/core:       115 |                 logger: new Logger(),
@vendure/core:       116 |             });

@michaelbromley
Copy link
Member

Ah yeah it's not exported. You can add to the core/src/index.ts:

export * from './i18n/i18n.service';

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

I would also like to export the I18nError to create own errors.

@michaelbromley
Copy link
Member

ok, you can create an i18n/index.ts file and re-export both from there, as is done in the other sub-directories.

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

Okay, only thing left is to improve the e2e test I guess.

Is there a way to force a language in a request? I don't quite get how the I18nRequest determines what language should be used with the TFunction

@michaelbromley
Copy link
Member

Yes, you pass the languageCode=de as a query parameter with the request. In an e2e test this is the 3rd param of the client.query() method:

const { product } = await adminClient.query<GetProductSimple.Query, GetProductSimple.Variables>(
GET_PRODUCT_SIMPLE,
{ slug: 'curvy-monitor' },
{ languageCode: LanguageCode.de },
);

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

Actually the query needs the parameter lang to work.

See:

lookupQuerystring: 'lang',

Is this correct?

@michaelbromley
Copy link
Member

Oh, yeah - the languageCode query string is actually used to determine the language of translatable entities like products, collections etc. I think you just found a bug right there - I guess if we change that setting to langaugeCode too, then i18next can use the same one. That would make sense.

@Draykee
Copy link
Contributor Author

Draykee commented Apr 12, 2021

Okay I'll change that to langaugeCode then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants