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

#419 - Added support for components localization #479

Closed
wants to merge 2 commits into from
Closed

#419 - Added support for components localization #479

wants to merge 2 commits into from

Conversation

hanc2006
Copy link

@hanc2006 hanc2006 commented Jul 9, 2021

First draft for add component localization support using directive and locale. Discussion here: #419 (comment)

@vercel
Copy link

vercel bot commented Jul 9, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/shoelace/shoelace/5TEPyXWS2AFgvzqnPBbwUHREbpgt
✅ Preview: https://shoelace-git-fork-hanc2006-i18n-shoelace.vercel.app

@hanc2006 hanc2006 changed the title Added support for components localization #419 - Added support for components localization Jul 9, 2021
@mcjazzyfunky
Copy link

mcjazzyfunky commented Jul 10, 2021

[Edit] tl;dr: "Singleton object" is not an anti-pattern per se. But it is often an anti-pattern when there are inflexible and unnecessary tight couplings, in our case especially with components. I think we should avoid singletons whenever possible.


@hanc2006 @claviska Hope you do not mind if again, I want to point out that singleton patterns are anti-patterns in most cases when dealing with i18n, due to the fact that many aspects of i18n may be contextual. Also trying to fix those singleton pattern issues, like I've proposed with this t.formatDate(this, date, format) misconception the other day, is a complete dead-end. Also using a singleton dictionary object with localizable texts will turn out to be suboptimal sooner or later [Edit: at least if it is tightly couple to components] (I do not know whether I have proposed that, but at least I have considered it as possible solution a couple of days ago).
Please allow me to respond to two of your comments to emphasize that (frankly, I haven't responded earlier because I was a little afraid of sounding kinda unfriendly or whatever):

This similar to how the Animation Registry works and seems reasonable.

To respond openly: No, it's not [Edit: at least not, if tightly coupled to components]. It's neither "similiar" nor "reasonable". The animation registries used by SL use an internal logic that shall never be changed, there are no (or at least no common) real-world scenarios where the internal behavioral strategy shall differ in certain contexts. Same for the singleton SL icon library registry, this is also completely noncontextual (fun fact, in the past there was this sl-icon-library component and when I saw it first, I indeed thought, the reason why it existed in the first place, would be to allow contextual icon management - turned out, I was wrong 😉). The requirements are completely different for I18n.

Turning a singleton into an instance for each [...] is done quickly

If we are true to ourselves, if that was really "done quickly" we would not have used singleton patterns in the first place. I personally find the whole i18n stuff and how to abstract from it properly extremely difficult.

For example the strategy of what shall be done if a translation cannot be found may differ completely between apps (one app may show the English version as fallback, a second may print out a warning and show ??? as placeholder, third may just throw an exception and a fourth app may just want to use that strategy that is already configured in the there used I18n library xyz). Same with most other aspects of i18n.

Fixing those isses with patterns like the following (like already mentioned), will be a dead-end:

// anti-pattern!
const PublicSingleton = {
  setStrategy1(strategy: Strategy1) { ... },
  setStrategy2(strategy: Strategy2) { ... },
  doSomething(...) {...}
  doSomethingElse(...) ...}
}

// anti-pattern!
const PublicSingleton2 = {
  doSomething(strategy: Strategy1, ....) { ... },
  doSomethingElse(strategy: Strategy2, ....) { ... }
}

I really believe we should not spent too much time on things like that. I know it will cost several more hours more of brain power for all of us and also more annoying discussion, but in the end it will have been worth while (at least I hope so 😄).

@hanc2006 hanc2006 mentioned this pull request Jul 22, 2021
@hanc2006
Copy link
Author

Closed in favor of this new PR #487.

@hanc2006 hanc2006 closed this Jul 23, 2021
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

Successfully merging this pull request may close these issues.

2 participants