Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

Issue with react-intl's context and shouldComponentUpdate #1

Open
gpbl opened this issue Jan 26, 2015 · 14 comments
Open

Issue with react-intl's context and shouldComponentUpdate #1

gpbl opened this issue Jan 26, 2015 · 14 comments
Labels

Comments

@gpbl
Copy link
Owner

gpbl commented Jan 26, 2015

After fetching the new locale data, components using shouldComponentUpdate (as those using the React.addons.PureRenderMixin) may block the update of the components, since they are not aware of the changed context. This problem is described here.

@gpbl gpbl added the bug label Jan 26, 2015
@slorber
Copy link

slorber commented Feb 27, 2015

Haha exactly, this is what I thought.

Nice try but your idea seems to work only when you have a single component unfortunatly :(

@gpbl
Copy link
Owner Author

gpbl commented Feb 27, 2015

I went on with the extended PureRenderMixin as for this issue, facebook/react#2517, eventually I'd fallback to props. I'm open to find better alternatives :-)

@slorber
Copy link

slorber commented Feb 27, 2015

props is just not manageable for my very large SPA :( I'll have to reload the full page for now :(

@gpbl
Copy link
Owner Author

gpbl commented Feb 27, 2015

Let's go for the reload :-) @caridy is right when he says the level of complexity raised by this is not worth the hassle.

@slorber
Copy link

slorber commented Feb 27, 2015

I've found a workaround:
facebook/react#3038 (comment)

:)

@caridy
Copy link

caridy commented Feb 27, 2015

👍 reload for the win!

@gpbl
Copy link
Owner Author

gpbl commented Mar 30, 2015

In this issue we are discussing an approach to skip the context using a Flux store. By listening to a Flux store, we should be able to change the locale without reloading the page.

@eplawless
Copy link

At Netflix we have this same problem, where e.g. if voice gets enabled on an XBOX, or if our locale changes, or if we switch to a kids' experience, we want to force a global re-render without respecting shouldComponentUpdate.

There are lots of external events across all of our platforms (web, mobile, TV) which should have this behaviour.

We've currently got a flawed solution in place using context. We've inserted a layer underneath shouldComponentUpdate (by forcing components to implement shouldComponentUpdateOverride instead) which forces an update if context.forceUpdate is or was set, or if shouldUpdateChildContext is a function returning true (inspired by facebook/react#2517).

It would be really nice to have a forceGlobalUpdate, forceUpdateRecursive, or similar. The context solution forces more re-renders than we'd like, and is very much a kludge.

@caridy
Copy link

caridy commented Mar 31, 2015

/cc @sebmarkbage, can you chime in here?

@sebmarkbage
Copy link

Yea, this is broken and one reason context is undocumented. I think that future versions of React will include an explicit determination of a context key change which will deeply force and update of every component that reads from that context key.

Context will be completely removed from shouldComponentUpdate since it doesn't make sense.

On Mar 31, 2015, at 11:45 PM, Caridy Patiño notifications@github.com wrote:

/cc @sebmarkbage, can you chime in here?


Reply to this email directly or view it on GitHub.

@caridy
Copy link

caridy commented Apr 1, 2015

cool, thanks @sebmarkbage. guys, as you can see, we will get this for free with the current implementation of react-intl, no magic is needed, just wait until react gets refactored.

@eplawless
Copy link

Thanks @sebmarkbage, looking forward to that update.

@slorber
Copy link

slorber commented Apr 6, 2015

@sebmarkbage agree with that solution. Since React has contextTypes for each componant using context you could easily know which components to re-render.

@eplawless have you checked my workaround here?
facebook/react#3038 (comment)
You can see hot language switch in action in our application stample.co if you want (after signup)

@tricoder42
Copy link

I solved this problem using subscribe/listen pattern in js-lingui.

HOC InjectI18n provides active language and message catalog for wrapped components. On mount, it subscribes its own forceUpdate. When ProvideI18n component receives new messages or language, it notifies all subscribed components about the change.

So, even if any component above return false in shouldComponentUpdate, the translated text is still updated. Only requirement is, that ProvideI18n component must be above any one with shouldComponentUpdate, obviously. These providers are usually top most components, so it shouldn't be a problem.

Cool thing is that even the Component which uses Trans component (translated message) don't need to update itself and still all translated texts inside will update on change of messages/language:

ProvideI18n -> ... -> Component with multilingual text -> InjectI18n(Trans)
                      ^-- This component and any above doesn't need to update

The subscribe pattern is really simple, you can see it here: https://github.com/lingui/js-lingui/blob/master/packages/lingui-react/src/I18nProvider.js#L64

I also used the same pattern when experimenting with hot reloading of message catalogs (https://github.com/lingui/js-lingui/blob/master/packages/example-usecase/src/App.js#L27) but I have a feeling, there must be a simpler way to do it :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants