Skip to content
This repository has been archived by the owner on Dec 15, 2018. It is now read-only.

Include parsed query in all history listener dispatches #240

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

aweary
Copy link
Contributor

@aweary aweary commented Oct 30, 2017

Second part of the fix outlined in #211 (comment)

@tptee I couldn't figure out an easy way to test this. enhancer.spec.js is mocking dispatch for the store created by the createStore call in the test file, but it isn't mocking the store that's creating inside the enhancer. To test this we'd probably want to somehow mock that internal store and assert that it's dispatching the correct payload for POP actions, but I can't figure out a clean way to do that 馃 That history.listen callback depends on the dynamic currentMatcher variable in the parent scope so refactoring is kind of tricky.

@aweary aweary requested a review from tptee October 30, 2017 23:37
@codecov
Copy link

codecov bot commented Oct 30, 2017

Codecov Report

Merging #240 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #240      +/-   ##
==========================================
+ Coverage   98.44%   98.44%   +<.01%     
==========================================
  Files          19       19              
  Lines         257      258       +1     
==========================================
+ Hits          253      254       +1     
  Misses          4        4
Impacted Files Coverage 螖
src/enhancer.js 94.44% <100%> (+0.32%) 猬嗭笍

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 2dbd876...1a4e2b5. Read the comment docs.

Copy link
Contributor

@tptee tptee left a comment

Choose a reason for hiding this comment

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

Awesome! I'll have to do some thinking on how we can test that inner store. The whole enhancer is kinda gnarly.

@tptee tptee merged commit 207222c into master Oct 31, 2017
@tptee tptee deleted the pass-query-to-location-did-change branch October 31, 2017 18:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants