adamduncan / eleventy-plugin-i18n

Eleventy plugin to assist with internationalization and dictionary translations
MIT License
103 stars 10 forks source link

Allow translations reload during serve #18

Closed izanagi1995 closed 1 year ago

izanagi1995 commented 3 years ago

Hello,

I wrote this little bit of code to allow a function to be passed in the translations property. With the help of the events triggered by 11ty, I can reload the translations before every build in serve or watch mode.

adamduncan commented 3 years ago

Thanks for the suggestion, @izanagi1995. Certainly getting translations to update without the need to restart the serve would be ideal.

There's an open issue related to this, which would ensure that the file (required in project's .eleventy.js config) reloads as one would expect: https://github.com/11ty/eleventy/issues/1312. Would love for that to be fixed at the library level, without having to bake workarounds into the plugin, so keeping a keen eye on the upcoming v1 release.

In terms of the PR, perhaps making the translations object dual-purpose (i.e. function or object) is a little ambiguous, API-wise? Do you have an example of what you'd call in your project's translations() plugin option to get the desired result?

I wonder if perhaps an explicit onBeforeWatch option for the plugin might be a promising approach.

izanagi1995 commented 3 years ago

Hi @adamduncan,

Thanks for the feedback. Having the issue fixed in Eleventy is a great news.

About the usage, I have several JSON files containing the translations for a lot of pages. I load them in a function using fs and combine the JSONs into a single objects as eleventy-plugin-i18n expects it. But it can be used in any other way (like pulling from an API).

I also understand the ambiguity of making the translations dual-purpose. However, this is a simple addition, so it doesn't break the current usage while keeping the API simple. if this is not a great idea for you, what about adding a new property called translationsFn which clearly indicates a function. But it would require an assertion to make sure one and only one translation property is used.

Exposing onBeforeWatch is not a great idea imho. It would add complexity to configure the plugin for not a lot of use cases.

adamduncan commented 3 years ago

Thanks for more info. I see where you're coming from. Would like to put a bit more thought into this one. Not sure workarounds in the API are the right approach just yet, in lieu of https://github.com/11ty/eleventy/issues/1312. I'll give it some more thought, if you're happy using your forked version for now 👍

Exposing onBeforeWatch is not a great idea imho.

Yep, fair point.

izanagi1995 commented 3 years ago

Alright, I hope that https://github.com/11ty/eleventy/issues/1312 will be solved soon (open for almost a year now). During that time, I will try to use my fork.

izanagi1995 commented 1 year ago

I deleted the repo by mistake. I asked GitHub to restore the repo