ficusjs / ficusjs-i18n

Functions for managing translations and localization in FicusJS components
MIT License
1 stars 1 forks source link

Implementing a class-wide, parallel-safe callback mechanism #59

Open mesr opened 1 year ago

mesr commented 1 year ago

I would like to propose and, if approved, offer to implement a class-wide, parallel-safe callback mechanism for the I18n extension.

The current implementation of the I18n extension as a global singleton object means that multiple client components can access the same I18n instance concurrently (such as when multiple native DOM events are fired, when workers are used, or when components implement asynchronous sections of code). This situation, without additional ad-hoc measures, makes I18n instances prone to race conditions.

Furthermore, reactive components currently rely on the correct implementation of locale-changer components, leading to indirect mutual coupling between publishers and subscribers, additional dependency on the event bus facility, and duplication of similar portions of code, e.g.:

// Currently, each component must publish their locale-changing events
createCustomElement('locale-changer',
  withI18n(i18n,
    withEventBus(eventBus, {
      // ...
      buttonClicked (locale) {
        this.i18n.setLocale(locale)
        this.eventBus.publish('i18n:locale:changed', locale)
      },
      // ...
    })
  )
)

A class-wide, parallel-safe callback mechanism would bring the following improvements:

// App-wide callback frees locale-changer components of the responsibility to
// publish locale-changing events, and of their dependency on the event bus
i18n.subscribe(data => {
  eventBus.publish('i18n:locale:changed', data)
})
// Locale-changer components are now decoupled from subscribers and the event bus
createCustomElement('locale-changer',
  withI18n(i18n, {
    // ...
    buttonClicked (locale) {
      this.i18n.setLocale(locale)
    },
    // ...
  })
)

Complete backward compatibility would be maintained. An optional callback argument would be added to the setLocale(locale) and getLocale() methods, homologous to the callback argument already present on method detectLocale(callback). A single method subscribe(callback) would also be added to the I18n extension's public API.

Tests and documentation would be updated accordingly.

ducksoupdev commented 1 year ago

Thanks for your suggestion @mesr . I like the proposal and would be happy to review any PR submitted for this.