FormidableLabs / redux-little-router

A tiny router for Redux that lets the URL do the talking.
MIT License
1.04k stars 114 forks source link

[WIP] immutable.js support #246

Closed stefvhuynh closed 6 years ago

stefvhuynh commented 6 years ago

Extra files added for non-immutable users include lodash/get, lodash/omit, and src/util/data.js

Still need to do tests and types.

codecov[bot] commented 6 years ago

Codecov Report

Merging #246 into master will decrease coverage by 49.75%. The diff coverage is 35.29%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #246       +/-   ##
===========================================
- Coverage   99.26%   49.51%   -49.76%     
===========================================
  Files          20       21        +1     
  Lines         271      307       +36     
===========================================
- Hits          269      152      -117     
- Misses          2      155      +153
Impacted Files Coverage Δ
src/components/fragment.js 93.47% <100%> (-4.3%) :arrow_down:
src/util/data.js 100% <100%> (ø)
src/components/link.js 37.83% <100%> (-62.17%) :arrow_down:
src/middleware.js 11.11% <14.28%> (-84.73%) :arrow_down:
src/enhancer.js 14.28% <15.78%> (-85.72%) :arrow_down:
src/util/create-installer.js 20% <20%> (ø)
src/environment/browser-router.js 28.57% <28.57%> (-71.43%) :arrow_down:
src/reducer.js 33.33% <31.11%> (-66.67%) :arrow_down:
src/environment/express-router.js 33.33% <40%> (-66.67%) :arrow_down:
src/environment/hash-router.js 40% <40%> (-60%) :arrow_down:
... and 14 more

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 6165404...ffdb633. Read the comment docs.

codecov[bot] commented 6 years ago

Codecov Report

Merging #246 into master will decrease coverage by 49.75%. The diff coverage is 35.29%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #246       +/-   ##
===========================================
- Coverage   99.26%   49.51%   -49.76%     
===========================================
  Files          20       21        +1     
  Lines         271      307       +36     
===========================================
- Hits          269      152      -117     
- Misses          2      155      +153
Impacted Files Coverage Δ
src/util/data.js 100% <100%> (ø)
src/components/fragment.js 93.47% <100%> (-4.3%) :arrow_down:
src/components/link.js 37.83% <100%> (-62.17%) :arrow_down:
src/middleware.js 11.11% <14.28%> (-84.73%) :arrow_down:
src/enhancer.js 14.28% <15.78%> (-85.72%) :arrow_down:
src/util/create-installer.js 20% <20%> (ø)
src/environment/browser-router.js 28.57% <28.57%> (-71.43%) :arrow_down:
src/reducer.js 33.33% <31.11%> (-66.67%) :arrow_down:
src/environment/express-router.js 33.33% <40%> (-66.67%) :arrow_down:
src/environment/hash-router.js 40% <40%> (-60%) :arrow_down:
... and 14 more

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 6165404...b91815f. Read the comment docs.

stefvhuynh commented 6 years ago

@tptee, @ryan-roemer, looking for feedback on this approach. thanks!

ryan-roemer commented 6 years ago

@stefvhuynh -- Nice work so far!

Immutable newbie question -- is there any way for us to support immutable in our src without ever needing to import immutable?

stefvhuynh commented 6 years ago

@ryan-roemer, in this PR, I only have immutable listed as a devDependency. I think if someone wants to use immutable with this lib, they should be including it in their own deps. So really, it should be listed as a peerDependency in here but I don't want to force that on people who aren't using immutable.

Additionally, the way I have structured the code, immutable should not be imported if someone just imports from src as there shouldn't be a code path to any of the code in the immutable folder from src (though the opposite is not true since we need to share code).

More generally speaking, I needed to use the immutable lib to detect whether a data structure is an immutable object or not as well as to convert any pure js objects into immutable objects. We will probably also need the type definitions.