-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor. #34
base: master
Are you sure you want to change the base?
Refactor. #34
Conversation
test/spec/.eslintrc
Outdated
@@ -1,2 +1,2 @@ | |||
env: | |||
mocha: true | |||
jest: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the overrrides
feature of the root .eslintrc
for this
https://github.com/10xjs/form/blob/master/.eslintrc#L7
export {default as reducer} from './reducer'; | ||
export {default} from './connect'; | ||
// @flow | ||
export {default} from './createRelocation'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this redundant file to cut down on bundle-size. or use rollup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at this point it's so tiny as to be a non-issue. But a good point nonetheless.
src/createRelocation.js
Outdated
// eslint-disable-next-line | ||
UNSAFE_componentWillMount() { | ||
this.props.addChild(this.id, this.props.children, this.props.meta); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use componentDidMount
, and componentDidUpdate
to handle side-effects (see: reactjs/rfcs#26)
Hopefully we can use these instead of the lifecycle funcs that are on the chopping block.
Codecov Report
@@ Coverage Diff @@
## master #34 +/- ##
===========================================
+ Coverage 83.67% 95.65% +11.97%
===========================================
Files 5 1 -4
Lines 49 46 -3
Branches 8 2 -6
===========================================
+ Hits 41 44 +3
+ Misses 8 2 -6
Continue to review full report at Codecov.
|
Change
relocation
to use the context API available inreact^16.3
. This makes portals much more "declarative". Instead of dispatching an action you mount/render a component.First create an instance:
Then wrap your app in a provider:
Then have a place to consume generated components somewhere:
Then dispatch components into the aforementioned containers:
/cc @10xjs