Copy injector code from rbp and expose api #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ts for `reducerInjectors`, misc.
BenLorantfy
changed the title
BenLorantfy
changed the title
BenLorantfy
requested review from
julienben,
justingreenberg and
vineyardbovines
BenLorantfy
force-pushed
the
move-code-from-react-boilerplate
branch
from
3f4e31c to
b43f889
Compare
|
Hey @BenLorantfy fyi we've basically copy-pasted this code into a large monorepo setup and have validated that at least the raw Anything you need to help get this over the line? |
Collaborator
Author
|
Just waiting for a review |
Collaborator
Author
|
@justingreenberg @julienben @Gretzky Does anyone have a chance to review this? |
julienben
requested changes
Aug 29, 2019
Member
julienben
left a comment
julienben
left a comment
There was a problem hiding this comment.
Sorry for the delay! Here are some thoughts/comments.
Member
vineyardbovines
left a comment
vineyardbovines
left a comment
There was a problem hiding this comment.
I have all the same comments as @julienben
Member
vineyardbovines
left a comment
vineyardbovines
left a comment
There was a problem hiding this comment.
(left one extra comment)
julienben
reviewed
Aug 29, 2019
julienben
reviewed
Aug 29, 2019
julienben
approved these changes
Aug 29, 2019
Member
julienben
left a comment
julienben
left a comment
There was a problem hiding this comment.
Thanks for your diligence! Excited to see this come to fruition!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR:
createInjectorsEnhancer(new),injectSaga,useInjectSaga,injectReducer,useInjectReducer,forceReducerReload(new),SagaInjectionModespackage-lock.jsonfile. (This is where most of the "lines added" is coming from)Still left TODO (In subsequent PRs):
If I should do any of the above in this PR let me know, I just thought to keep this PR small-ish I would do these in subsequent ones.
A few other notes:
Pinning Dependencies in
package.jsonI don't know if this is a good idea for a library. I've been working recently on reducing my webpack bundle size, and I've noticed that pinned dependencies are more prone to being duplicated.
For example, if I pin "lodash" to
4.17.15and a consumer of this library has a "lodash" version of^4.20.0, this will cause two versions of "lodash" to be present in the bundle. If I had however specified^4.17.15, then one version would be present in the bundle.checkStore
This has been removed from the API.