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
Using react-intl with es6 classes #99
Comments
If you are using flux, you can keep the |
Yeah I'm using flux. I'm already keeping my messages + locales inside a store, since I want to be able to change languages on the fly. But I most certainly don't want to wrap each translatable component inside a higher order component. Having stuff on context seems a bit more convenient to me. Of course if there is no other way, then a higher order component will have to do. Also I don't necessarily want to copy the libraries functionality. That's why I asked for support from the devs ;) |
In my experience skipping context when using flux is not actually that big deal: instead of adding the mixin, you wrap the component. The |
Well the context is nice, because I don't want to have to pass down messages explicitly, if from grandparent to grandchild, if the parent does not need the messages. But your idea of externalizing Did you personally run into any issues with |
This is a good discussion! I'd like to provide a sans-mixin way of using all the features of React Intl. We're actively working with teams at Yahoo to figure out good patterns for translation fallbacks (for things like missing translations, etc.) Let's keep this discussion going and continue to expiriment with solutions has you two have already. |
@ericf I went with what @gpbl suggested and externalized the So in the wrapped component translations work rather normally: render() {
const getIntlMessage = this.props.getIntlMessage;
const msg = getIntlMessage('some.message.path');
} For fallbacks my idea was to always provide a complete default set in a TranslationStore, and then upon language change merge the new language with the default set and return that. That way at least the english version of the message will show up. And if it's missing, then the original english version could even decorated with a clear suffix, that this string is missing (might be useful for dev mode). Another idea would be to return the path identifier, with some added prefix or suffix whenever something is not defined. |
My implementation is a bit different. I'm using Fluxible, so I'm cheating when I say I skip Said I use // utils/connectToIntlStore.js
import React from 'react';
import { FluxibleMixin } from 'fluxible';
function connectToIntlStore(Component) {
const Connection = React.createClass({
mixins: [FluxibleMixin],
render() {
const { messages, locales } = this.getStore('IntlStore').getState();
return <Component messages={messages} locales={locales} {...this.props} />;
}
});
return Connection;
}
export default connectToIntlStore; This is a wrapped component: import { Component } from 'react';
import connectToIntlStore from '../utils/connectToIntlStore';
import getIntlMessage from '../utils/getIntlMessage';
class MyComponent extends Component {
render() {
const { messages } = this.props;
return (
<p>
{ getIntlMessage(messages, 'MyComponent.helloWorld') }
</p>
);
}
}
export default connectToIntlStore(MyComponent);
// MyComponent has `messages` and `locales` as props. Basically, instead of using the react-intl
I'm still experimenting with this, anyway. @johanneslumpe I like the idea of the fallback to the english locale – but how would that work? Would a foreign user download two languages, just because a missing string? I'm convinced the best practice is to stop the build when the locale strings don't match. The issue with contexts and |
So you're fetching the messages from your store for each component. I'm fetching them once at the top and then I'm relying on the normal context. Here's what I use as mixin replacement: export default (Component) => {
return class Translatable extends React.Component {
static contextTypes = {
messages: React.PropTypes.object
}
constructor(props, context) {
super(props, context);
// we bind `getIntlMessage` so we can just use it normally inside
// `render`, without having to always pass the component instance
this.getIntlMessage = getIntlMessage.bind(null, this);
}
render() {
return <Component getIntlMessage={this.getIntlMessage} {...this.props} {...this.state} />;
}
};
} And the usage is as follows: class Comp extends React.Component {
render() {
const getIntlMessage = this.props.getIntlMessage;
return (
<div>
{getIntlMessage('some.path')}
</div>
);
}
}
export default makeTranslatable(Comp); @gpbl Yeah I've read that thread about the context. I didn't encounter it yet. But if I do, I might have to switch to props, instead of context. But I'm confident that the context issue will be solved in time. About the fallback: Yeah, a simple version would include both language sets. Another way would be to generate language files on the server, and do the merging prior to serving (potentially cached). |
Right! Your solution works better for changing the language on the fly: attaching a store listener to each component could be too much :) – on the other side, I am not sure about that
What about the Another part I couldn't replace with the high order component was the use of |
Yeah don't use those functions, just go with the components. I'm currently not using the My I chose to pass |
In my demo app i tried to skip the Then i got the idea to wrap react-intl components into an high-order component so that they always use the values coming from the store. For example, a ...but here i have a problem with react-intl contexts, when a wrapped components get another wrapped component as prop, for example here: <WrappedFormattedMessage
message="photo.rating"
rating={<WrappedFormattedNumber value={rating} />}
/> where I get a warning from react:
maybe @ericf could suggest what's going wrong here? |
So I've begun a major refactor to try to address many of the things brought up in this thread and others. I'm still trying to work through the details and flush out an implementation to propose, while also attempting to preserve v1.x backwards compatibility. Here's the rough set of things I'm trying to address:
I first attempted to create a new import Intl from './components/intl';
export default function provideIntl(Component) {
return class extends Intl {
render() {
return React.createElement(Component, this.props);
}
};
} This approach gets around the current state of React only having owner-based contexts; and it would be used like this: AppComponent = provideIntl(AppComponent);
React.render(
<AppComponent locale="en-US" messages={{...}} />,
document.getElementById('container')
); This new HOC @johanneslumpe you're right about adding I'm then extrapolating these concepts out to provide support for fallbacks. What we realized is that in order to correctly provide support for falling back to the app's default locale when a translation is missing we need to combine AppComponent = provideIntl(AppComponent);
React.render(
<AppComponent locale="fr-FR" messages={{...}}
fallback={{
locale: 'en-US',
messages: {...}
}} />,
document.getElementById('container')
); (Still working on the exact API and naming, but you get the idea.) So now the I've also worked on updating the var format = this.context.format;
var props = this.props;
var message = this.props.message;
var key = this.props.key;
if (typeof message !== 'string') {
if (!key) {
throw new Error(
'A `message` or `key` path to a message must be provided.'
);
}
try {
message = format.getMessage(key);
} catch (e) {
try {
// Fallback.
format = this.context.fallbackFormat;
message = format.getMessage(key);
} catch (e) {
throw new Error(
'Message could not be located at key: ' + key
);
}
}
} I'm still unsure if adding One other open question I have is around what to do with the This is sort of a rant at this point, but since you two are already thinking about this stuff, it makes sense to flush some of this out here first before I open a RFC and/or branch to discuss the details future 😄 |
@ericf nice start :) First thing: I don't think you can use As for the HOC name, how about just Instead of using a HOC factory, we could have a component like the I think with that we could solve the context issue of providing a context to children of the I18n component. Also messages + locales and fallbacks could be made directly available as params to the custom render function (not sure if we need that, but it would be possible), so they could be manually passed down to basically any component inside that render function. So each time a component needs direct access to the I'd like the fallback to be configurable. Instead of throwing an error, if there is no message and no fallback, I'd like to be able to specify that just the key gets output again. This would make it easy (while in dev-mode) to see where something is missing, taking a note and continue to check, instead of the app breaking. And in addition to that we could log using Have no idea about the |
Ah good point!
So you're thinking that we could go with this style: <Intl locale="en-US" messages={{...}} render={() => {
return <AppComponent />;
}} /> And then when React switches to parent-based context, the refactor would be to the following? <Intl locale="en-US" messages={{...}}>
<AppComponent />
</Intl> I could make the above "work" today using
Yeah, that's a cool idea! But maybe we'd just pass the
Yeah I'd like to figure out a good way to do that which doesn't complicate the story of using React Intl. I just started (locally) with keeping the same error semantics as there currently are. The other thing I'm trying to figure out is how to keep the <FormattedNumber value={0.95} locales={['en-US', 'en']} style="percent" /> (Note: using the plural Currently, each |
v0.13 introduces the new |
@PaulLeCam yeah I was hoping that would work, but it doesn't change the ownership of the element being closed, therefore it won't change the context inheritance chain. |
I just went down much the same path @ericf went already before if I read his comments correctly. I created an Intl React component that sets up a context which contains the locales, messages and formats. You use it somewhere on the outside of your application to wrap it, i.e:
I then created a special wrapper of FormattedMessage which does two things:
I ran into the same issue with ownership versus parents for context where I wanted to handle the children of Some questions:
I'm asking also because I may want to start using this approach faster than react-intl does, but I want to make sure react-intl is actually heading in this direction at all. |
+1 |
It's even worse than needing to use React.addons.cloneWithProps to make sure ownership is consistent with parentage. I actually can't get it to work -- I tried my best to replicate what FluxComponent from Flummox does, but I can't get it working. I think to make stuff like this work we need React 0.14 with a proper parent-based context. |
The changes @ericf is working on sound good and I'd like to be able to upgrade to a future major release without too much trouble. Given that, I'm wondering what those of you who have been working around mixins-with-es6-classes are currently using or would recommend. |
@necolas check this out: |
Thanks. I'm leaning more towards what @johanneslumpe is doing, rather than using a mixin directly on every component. @johanneslumpe please can you elaborate / share more of the implementation if it's working well for you. |
@necolas Sorry for the late reply - wasn't around. I actually haven't had the chance to continue my research on that topic. The current implementation is working in a way that you have I have a single root component, which does use the mixin to set up the The application itself has a single top level handler, which listens for changes in the That worked well for me so far. Always loading the default and the custom language set is of course not Does that help or do you need more details? :) |
Hey, thanks for the follow up. Specifically, you have this line in your earlier example: this.getIntlMessage = getIntlMessage.bind(null, this); Is this a custom implementation of |
@necolas Oh sorry, forgot to mention that. I probably should have included that up that. It's the |
Thanks. I had it working with the mixin method too. Was just curious if there was some extra stuff going on in there. Quite a nice solution for now. |
What I've done to use import {assign} from 'lodash';
import {IntlMixin from 'react-intl';
export default class Profile extends React.Component {
displayName = 'Profile'
contextTypes = {
router: React.PropTypes.func
}
static propTypes = {
flux: React.PropTypes.object.isRequired
}
_getIntlMessage = IntlMixin.getIntlMessage
_formatMessage = IntlMixin.formatMessage.bind(assign({}, this, IntlMixin))
... I can use (note: I use babel stage 0) |
@faassen I like your solution about creating a wrapper around FormattedMessage that looks up directly into the context. @ericf I think it would be great to have it integrated in the lib directly. Something like |
@slorber this is similar to what I'm doing now, but I don't use contexts – I wrapped the original <FormattedMessage message="some.message" /> https://github.com/gpbl/isomorphic500/blob/master/src/utils/FormattedMessage.js Here I've explained a bit my approach. |
would be great if the right way to use ES6 syntax is described in the README file |
To be honest, I don't think there's a right way to use the current version with ES6, if you want to have the full access to the API and not just the components. As all of our applications use react-router, the first route handler is a IntlApp component, injecting the context along with messages and formats, selected for the user locale. We have the same component for all of our applications. |
Hi @AlexJozwicki, do you have an example of you implementation? Regards. |
This is what's being worked on in the |
hey @ericf, I'm looking forward to 2.0 🎉 |
sorry for OT, but is there a timeline for releasing 2.0? |
@jakubsikora working on integrating/upgrading it into several Yahoo apps in production before shipping to work out all the kinks. I'm feeling good where it's at so nearing a preview release though. |
@ericf in ver 1 there is a method var foo = this.getIntlMessage('foo');
return (
<div>{foo}</div>
); |
@jakubsikora this will be one area I'll be looking for feedback on. The current design uses a simple, flat, key --> value hash to store the messages, but one of the bigger features for v2 is automatic translations fallbacks, implemented by this algorithm. |
Just for fun, what we've ended up doing is shipping a default language with the application (English in our case) and having that key/value hash live in a Flux store. Then we dynamically load other languages from S3 and put them into the same Flux store in an array. If a user wants to use a language other than English, we use lodash's merge function to combine one language key/value hash with English. The premise is that English will always be a step ahead and canonical. So if a key is missing in a newly loaded language, it easily falls back to English as a result of merging the two objects. |
@ryan1234 yup, this is our experience too! In v2, this idea is baked in, and if you're using the Babel and ES6 modules, then you'll be able to use the forthcoming |
See: #162 |
Updates the requirements on [@types/fs-extra](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/fs-extra) to permit the latest version. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/fs-extra) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: null <27856297+dependabot-preview[bot]@users.noreply.github.com>
Are you guys planning on providing a solution for this, like a higher order component for example? Or would you propose a different way of using react-intl without mixins?
How about putting the translation methods into
context
, together with the already present props? Then a higher order component could just prepare the context and render a given child. But instead of then having to mix in the react-intl mixin, we could just define the propercontextTypes
and have access to all the functionality we need.The text was updated successfully, but these errors were encountered: