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

Is the Mixin format judicious? #3

Closed
AlexGalays opened this issue Jul 21, 2014 · 2 comments
Closed

Is the Mixin format judicious? #3

AlexGalays opened this issue Jul 21, 2014 · 2 comments

Comments

@AlexGalays
Copy link

Hello,

Just some thoughts,
Is there any added value in making this a react mixin?

  • It seems it doesn't depend on any react features and doesn't particularly fit the 'component' concern. I have yet to find a good use case for mixins, but if there was one I would imagine it closely related to rendering/lifecycle.
  • Why not favor composition over inheritance (mixin)? Once imported, you would write intl.date instead of this.intlDate. Mixing many different concerns into one component also pollutes the dynamic this pointer which is known to be annoying (React bind it only for the top level functions of a component). With composition, the lib is no longer coupled to React components, and can be called from other places.

Cheers

@caridy
Copy link
Collaborator

caridy commented Jul 22, 2014

Ok, let me try to address does one at a time:

  • the low level API already exists, you can do Intl.DateFormat(), you can also use intl-messageformat module to implement what in the future could be Intl.MessageFormat() using ICU format. In general, those low level APIs are ok, but, there are few implications that a higher level API will help to address, which is what react-intl, handlebars-helper-intl and dust-helper-intl are trying to do. If you look at src/component.js, you will see that it uses the low level APIs under the hood.
  • react components will require ICU messages to be available to format them using the props, but those messages are bound to a locale, and same for formatters. the best way we have found to accommodate this is by sharing messages and formatters between react components as part of the composition process, that's what this mixin does it. You call for render on the top level component, you pass all messages, formatters, the the locale, and it makes those available for every child component, so you don't have to propagate the messages thru props.
  • it does depends on react context to propagate messages, formatters and locales so child components can use them.
  • formatting a date, a number, or a message is an expensive operation (even when using the native implementation in chrome, firefox, or any other browser that supports ECMA402), this abstraction adds a level of caching that improves the formatting process drastically (we are talking order of magnitud faster).

I understand that having standalone libraries and global structures for messages, locales and formatters could work just fine, but complex apps with tons of components and translation per component by professional translators will require some sort of coupling between components and the messages, including some sort of validation processes, and that's what this mixin is doing. If a component brings the mixin into the definition, it means it will require some machinery under the hood to guarantee that messages, formatters and locales are in place to facilitate the rendering of those messages.

@AlexGalays
Copy link
Author

Alright!

longlho pushed a commit that referenced this issue Apr 27, 2020
longlho pushed a commit that referenced this issue Apr 27, 2020
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