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

Ensured that store from props is initialized with initialize from props. #136

Merged
merged 2 commits into from
Nov 8, 2018

Conversation

thchia
Copy link
Collaborator

@thchia thchia commented Nov 8, 2018

This addresses the 'provider initialization failure' in #135 .

Problem

When both store and initialize are passed to LocalizeProvider, the store is not initialized with the passed payload.

If initialize is omitted, this problem does not occur because the initialize() function is called from userland (from the withLocalize-wrapped component). This explicitly dispatches the initialize action.

If store is omitted, this problem does not occur because there is already code to initialize the passed payload into LocalizeProvider's state.

Changes

I have added a method that initializes the passed store with the passed initialize payload (if both are present), and call it in componentDidMount. Since there are more things happening in componentDidMount as a result of this, I extracted the existing code (subscribe to store) to a separate method. I have also added a test.

I think some parts of this diff are caused by prettier not being run somewhere in the past. The new test is at the end of the test file.

@codecov
Copy link

codecov bot commented Nov 8, 2018

Codecov Report

Merging #136 into master will increase coverage by 0.07%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
+ Coverage    96.4%   96.47%   +0.07%     
==========================================
  Files           6        6              
  Lines         250      255       +5     
  Branches       76       77       +1     
==========================================
+ Hits          241      246       +5     
  Misses          8        8              
  Partials        1        1
Impacted Files Coverage Δ
src/LocalizeProvider.js 91.17% <87.5%> (+1.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b04f48...e87fc63. Read the comment docs.

Copy link
Owner

@ryandrewjohnson ryandrewjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@ryandrewjohnson ryandrewjohnson merged commit b7664b2 into ryandrewjohnson:master Nov 8, 2018
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

Successfully merging this pull request may close these issues.

2 participants