Skip to content

Double renders or provider initialization fails #135

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

Closed
JeffBaumgardt opened this issue Oct 26, 2018 · 4 comments
Closed

Double renders or provider initialization fails #135

JeffBaumgardt opened this issue Oct 26, 2018 · 4 comments
Labels

Comments

@JeffBaumgardt
Copy link

JeffBaumgardt commented Oct 26, 2018

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Same issue as #132. Double renders when using withLocalize

I followed the instruction to initialize the provider but the initialization does not seem to take affect. Leading to my strings not loading in at all when attempting to addTranslation and getTranslation is empty. As is the store.localize.translations. Also the other options i set in initialize are not present in the store. Please see attached codesandbox for simplistic example - https://codesandbox.io/s/j2jq5opmmv

If I use withLocalize to initialize the app I get translations and everything loads just fine but I get a double render of my parent component, causing children to re-render, even though no pros change - https://codesandbox.io/s/21mqp8qx0j

The second option while works causes side effects in cDM to run twice.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
No Initialize - https://codesandbox.io/s/j2jq5opmmv
Double Render - https://codesandbox.io/s/21mqp8qx0j

What is the expected behavior?
In either situation I would expect no double render with withLocalize or using the initialize to the provider to have it work as expected.

Which versions of react and react-localize-redux are you using?
React: 16.6
react-localize-redux: 3.5.0

@thchia
Copy link
Collaborator

thchia commented Oct 29, 2018

@ryandrewjohnson I think there could be a bug in the case when the following conditions are satisfied:

  1. store and initalize props are passed to LocalizeProvider
  2. localize state is accessed from redux store.

I think it is because there is missing initial state that needs to be set in the constructor of LocalizeProvider:

constructor(props: LocalizeProviderProps) {
    super(props);

    const dispatch = this.props.store
      ? this.props.store.dispatch
      : this.dispatch.bind(this);

    this.getContextPropsSelector = getContextPropsFromState(dispatch);

    const initialState =
      this.props.initialize !== undefined
        ? localizeReducer(undefined, {
            type: INITIALIZE,
            payload: this.props.initialize
          })
        : localizeReducer(undefined, ({}: any));

--> // Need something like this (maybe better in componentDidMount though...)
    if (this.props.store && this.props.initialize !== undefined) {
      dispatch({
        type: INITIALIZE,
        payload: this.props.initialize
      });
    }

    this.state = {
      localize: initialState
    };
  }

This would ensure that the initialize payload is present in the redux store (if both those props are passed). I'm not 100% if this is the correct reason though so prefer to have your input on it.

@thchia thchia added the bug label Oct 29, 2018
@ryandrewjohnson
Copy link
Owner

@JeffBaumgardt can you try the proposed solution I discussed here, and let me know if that works.

I still need to update the docs to instruct people to use the initialize prop on LocalizeProvider as the preferred initialization flow.

@JeffBaumgardt
Copy link
Author

That's what I tired in my first sandbox. I added the initialize prop to the provider but I'm not getting anything back out, redux isn't updated and no keys exist.

@ryandrewjohnson
Copy link
Owner

Fixed in #136 and released in v3.5.1

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

No branches or pull requests

3 participants