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

Roadmap to v2 #144

Closed
radekmie opened this issue Nov 30, 2021 · 14 comments
Closed

Roadmap to v2 #144

radekmie opened this issue Nov 30, 2021 · 14 comments

Comments

@radekmie
Copy link
Contributor

radekmie commented Nov 30, 2021

This issue will be updated later.


Our main goal is to make the package lighter, more configurable, and easier to debug.

  1. Lighter
  2. More configurable
  3. Easier to debug
  4. Future ideas
    • Create and maintain a proper changelog from v2.0.0 on.
    • Setup CI process.
      • With automated publishing, if possible.
    • TypeScript-safe translations, especially for getTranslation.
      • Maybe with a runtime validation for missing translations?
    • Provide an integration with the Intl API?
    • Consider adding a proper support for pluraziation.
@radekmie radekmie added this to Needs triage in Open Source (migrated) via automation Nov 30, 2021
@piotrpospiech piotrpospiech moved this from Needs triage to In progress in Open Source (migrated) Dec 3, 2021
@Gwened
Copy link

Gwened commented Jan 5, 2022

Why would you want to remove the purify option?

Sometimes you want to translate some HTML texts.
Some use cases that come to mind:

  • Email templates
  • Having <b></b> as part of text data to let writers emphasis some keywords.

The default purify function html-encodes data, so it's not usable as HTML: the tags appear as text.

Currently, you can use this trick to bypass it:
i18n.setOptions({purify: (s: string) => s});
But that global setting is ugly, while, prior to v1.32, you could conveniently pass _purify=false to specific texts.

What do you intend to do regarding those use cases?

@radekmie
Copy link
Contributor Author

radekmie commented Jan 6, 2022

Hi @Gwened and thanks for taking the time for feedback!

We want to remove this option, as it doesn't make sense to have all translations purified. As you said, sometimes you want to translate email templates or other potentially data-sensitive texts. In our opinion, you should sanitize (purify) the text explicitly, in your code, according to your own rules and requirements. If you need one, I strongly recommend the sanitize-html package - we do use it in several projects and it serves us very well. Just keep in mind that it's relatively large.

Additionally, our current implementation yields unnecessary performance impacts that could be avoided.

@Gwened
Copy link

Gwened commented Jan 6, 2022

Thanks, makes sense.
So from now on, I'll use the options to bypass it globally, and call my own purify fonction whenever dealing with user content or requiring to print HTML code.

@piotrpospiech piotrpospiech mentioned this issue Feb 3, 2022
@jasongrishkoff
Copy link

jasongrishkoff commented May 6, 2022

Hi, I can see there's a Readme for v2 somewhere, but I'm unable to access it. Can you possibly make this public?

@piotrpospiech
Copy link
Contributor

Hi, I can see there's a Readme for v2 somewhere, but I'm unable to access it. Can you possibly make this public?

Hi @jasongrishkoff, Readme was changed a little, but it is still available on the main page of the Github repo. Are you looking for something specific?

@jasongrishkoff
Copy link

From what I can see, one of the main changes is that you've removed purify, but the docs still reference is. Will it still work with this new version?

@piotrpospiech
Copy link
Contributor

purify was removed. It won't work anymore. I will update Readme.

@jasongrishkoff
Copy link

Okay great. And the issue with onChangeLocale on <T> has also been resolved? Was poking through the code for a reference to this but didn't see it.

@piotrpospiech
Copy link
Contributor

What issue are you referring to? Can you send me a link?

@jasongrishkoff
Copy link

Sorry for the confusion. It was this comment: https://forums.meteor.com/t/what-are-currently-the-most-optimal-i18n-tools/57865/6

@piotrpospiech
Copy link
Contributor

We described several ways to integrate this package with React. There is an approach with the meteor's ReactiveVar and useTracker. I think this is what you are looking for. You can read about it in the Readme at the very bottom of the "Integration with React" section.

@dovydaskukalis
Copy link

dovydaskukalis commented Jul 28, 2022

2.0 RC1 does not work and the build process crashes with ENOENT: No such file or directory on Windows because of 2 test files (meteor_practicalmeteor:sinon.d.ts and meteor_universe:i18n.d.ts) having a colon in the file name which is not allowed on Windows. Replacing it with _ fixes it but not sure if it affects anything else.

@piotrpospiech
Copy link
Contributor

Hi @dovydaskukalis, thanks for your feedback. I fixed that in v2.0.0-rc.2

@piotrpospiech
Copy link
Contributor

v2.0.0 has been released!

Open Source (migrated) automation moved this from In progress to Closed Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

6 participants