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

redux-subspace version 2 - Spliting packages, better middleware support, redux-saga support, improved docs #34

Merged
merged 63 commits into from Aug 15, 2017

Conversation

mpeyper
Copy link
Contributor

@mpeyper mpeyper commented Aug 7, 2017

This one is huge. I don't think a single line of the version 1 survived. It will probably be easier to just look at or clone my fork than to read this PR, but review however you feel is easiest for you.

The changes here are plentiful:

  • Complete refactor of core subspace code
    • Middleware support
      • Improved support for Redux middleware
      • Added middleware pipelines for subspaces as well
    • More versatile ways to create subspaces (less duplication in keys for common cases)
  • Library has been split into multiple packages
    • redux-subspace: The core of the library
    • react-redux-subspace: React bindings to create subspaces for components
    • redux-subspace-saga: Support for creating subspaced sagas compatible with redux-saga
    • redux-subspace-wormhole: A redux-subspace middleware that replaces the root node functionality
  • Improved documentation

mpeyper and others added 30 commits July 3, 2017 00:24
subspaced actions can now fire more than once
Still need to fix READMEs and examples
Haven't made api changes yet
Fixed package.json of react-redux-subspace
mapState is now optional: default to `(state) => state[namespace]` (from ioof-holdings#32)
Removed `root` node of sub-state - can use middleware
Fixed typescript tests (they would never fail before)
@vivian-farrell
Copy link
Contributor

Is the import in https://github.com/mpeyper/redux-subspace/blob/master/packages/react-redux-subspace/src/components/SubspaceProvider.js for subspace correct? Redux-subspace is not in the list of packages in the package.json so how is it resolved?

@vivian-farrell
Copy link
Contributor

My bad, it is in the list. Ignore the comment above.

Copy link
Collaborator

@jpeyper jpeyper left a comment

Choose a reason for hiding this comment

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

The code looks good and test coverage is awesome. Good job.

Docs also seem good, although I think there may be some tweaking over time.

I think we should merge this as is and then fix any docs problems on master.

@vivian-farrell
Copy link
Contributor

I'm happy to merge this. I think we should publish to NPM with a canary build until we're happy it's stable.

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

Successfully merging this pull request may close these issues.

None yet

4 participants