✨ migrate to k-ramel #123

bpetetot · 2018-02-10T19:02:44Z

Closes #104

fabienjuif · 2018-02-11T11:35:27Z

How do you feel this migration right now ?

bpetetot · 2018-02-11T14:01:05Z

I am doing the migration in several steps :

1. apply createStore and inject of k-ramel (without refactoring) (836087d)
works directly with hideRedux=false ❤️
2. apply hideRedux=true and make it working (38818df)
3. replace redux reducers by k-redux-factory
4. replace saga by listeners progressively and refactor actions names (it's messy today)
5. apply listeners closer to the screens

fabienjuif · 2018-02-12T07:44:25Z

src/screens/organizer/proposals/proposalFilters/proposalFilters.container.js

@@ -1,19 +1,20 @@
-import { connect } from 'react-redux'
+import { inject } from 'k-ramel/react'


I am thinking about publishing x distincts packages:

k-ramel

@k-ramel/react

@k-ramel/driver-http

@k-ramel/...

What is your though ?

Yes it's a good idea, but I think you should keep an unique github repository with sub-packages

Yes that is the idea :)

fabienjuif · 2018-02-13T15:46:57Z

src/screens/speaker/myTalks/myTalks.container.js

@@ -9,7 +9,7 @@ import MyTalks from './myTalks'
 const mapStore = store => ({
  loaded: store.ui.speaker.myTalks.isInitialized(),
  talks: store.ui.speaker.myTalks.getAsArray(),
-  load: () => store.dispatch({ type: 'ON_LOAD_SPEAKER_TALKS' }),
+  load: () => store.dispatch({ type: '@@ui/ON_LOAD_SPEAKER_TALKS' }),


You can dispatch('@@ui/ON_LOAD_SPEAKER_TALKS') now if you want :)

OMG ! I forgot that. its fantastic !

bpetetot · 2018-02-13T17:38:48Z

@fabienjuif @guillaumecrespel

k-ramel migration finished and saga out !
About 1/3 of code deleted

Now I must do some tests to see if I have some regression, but the migration was not difficult and progressive.

guillaumecrespel · 2018-02-14T06:35:30Z

is it possible to use your example in our talk ?

fabienjuif · 2018-02-14T08:11:39Z

There still have some refactoring to do @bpetetot
I think about drivers.
You use a lot of dispatch in your reactions.

We have a custom driver (that we have to open source) for redux-form, it is like this:

import { setSubmitSucceeded, getFormValues, setSubmitFailed, startSubmit, stopSubmit, isSubmitting } from 'redux-form'

const asyncSubmit = (name, dispatch, getState) => async (http, ...options) => {
  dispatch(startSubmit(name))
  await http(...options)
  if (isSubmitting(name)(getState())) dispatch(stopSubmit)
}

export default ({ dispatch, getState }) => name => ({
  getFormValues: () => getFormValues(name, state => state.ui.form)(getState()),
  setSubmitFailed: (...fields) => dispatch(setSubmitFailed(name, ...fields)),
  setSubmitSucceeded: () => dispatch(setSubmitSucceeded(name)),
  startSubmit: () => dispatch(startSubmit(name)),
  stopSubmit: errors => dispatch(stopSubmit(name, errors)),
  asyncSubmit: (http, ...options) => asyncSubmit(name, dispatch, getState)(http, ...options),
})

And can be use like this for example:

export const login = reaction((action, store, { http, form }) => {
  // retrieve credentials from redux
  const signin = form('signin')
  const values = signin.getFormValues()
  // call API
  signin.asyncSubmit(
    http('AUTH')
    '/api/login',
    {
      method: 'POST',
      body: JSON.stringify(values),
      headers: { 'Content-Type': 'application/json' },
    },
  )
})

bpetetot · 2018-02-14T09:41:28Z

Yeah ! that's great ! and yes I have a lot of refactoring to do :

use redux-form driver ❤️
find a way to manage data loading on routes and component mounting. Each time I have an URL with param eventId, I have to do a loader like that, I think I could factorize this code in a better way.

const mapStore = (store) => {
  const eventId = getRouterParam('eventId')(store.getState())
  const event = store.data.events.get(eventId)
  return {
    loaded: !!event,
    eventId,
    load: () => store.dispatch('@@ui/ON_LOAD_EVENT'),
  }
}
export default compose(
  forRoute.absolute('PROPOSALS'),
  inject(mapStore),
  loader,
)(Proposals)

maybe create a driver to have a simpliest use of the router in reaction :

 // replace
 const eventId = getRouterParam('eventId')(store)
 // by
 const eventId = router.getParam('eventId')

 // replace
 store.dispatch(push(`/organizer/event/${eventId}`))
 // by
 router.push(`/organizer/event/${eventId}`)

rename some action names, it's still messy in my mind sometimes

fabienjuif · 2018-02-14T09:54:44Z

For the second point, look at the new hoc listen.
Maybe we can dispatch an event in this hoc like @@krml/LISTEN_PLUGUED>${componentName}

And in the listeners you plug with listen hoc, you catch that ?

bpetetot · 2018-02-14T11:15:23Z

@guillaumecrespel yes you can ;)

fabienjuif · 2018-02-14T16:30:51Z

src/screens/speaker/talk/edit/talkEdit.container.js

 const mapStore = (store) => {
  const talkId = getRouterParam('talkId')(store.getState())
  const talk = store.data.talks.get(talkId)
  return {
    loaded: !!talk,
-    form: FORM_NAME,
+    form: 'talk-edit',


You are throwing away the optimisation with a string here I think.
This was better to make it a constant so the string reference is always the same and the shallowCompare works better

fabienjuif · 2018-02-14T16:33:42Z

src/store/drivers/reduxForm.js

@@ -0,0 +1,25 @@
+import {


You should wait k-ramel driver before merging IMO

Yes I will wait for it ;)

fabienjuif · 2018-02-14T16:34:01Z

src/store/listeners/reactions/events.js

-    store.dispatch(stopSubmit(FORM, { _error: error.message }))
-    throw error
-  }
+export const createEvent = reaction(async (action, store, { form }) => {


This is better !

fabienjuif · 2018-02-14T16:34:40Z

src/store/listeners/reactions/events.js

+    // update event into database
+    updateForm.asyncSubmit(eventCrud.update, event)
+    // update event in store
+    store.data.events.update(event)


Don't you want to await asyncSubmit ?

No, I'm an optimistic guy 😃

fabienjuif · 2018-02-14T16:35:33Z

src/store/listeners/reactions/talks.js

@@ -1,51 +1,30 @@
 import { reaction } from 'k-ramel'


I think we agree, but this file should be attach with listen hoc no ?
And others of same types

Yes it will be an other refactoring step...

fabienjuif · 2018-02-17T10:28:18Z

❤️

🎉 basic migration

836087d

bpetetot added 11 commits February 11, 2018 17:23

🔥 apply hideRedux to true

38818df

simplify data store

c10eff2

migrate auth reducer to k-redux-factory

cdee38f

change organizer reducers to k-redux-factory

b0c263c

change organizer reducers to k-redux-factory

15d655b

update speaker reducers with k-redux-factory

b9c0ead

separate firebase from sagas

2296de8

rename redux folder to store

6d32e51

move reducers and listeners

a005a94

move user saga to listeners

8f4e849

migrate speaker saga

bd634f1

fabienjuif reviewed Feb 12, 2018

View reviewed changes

fabienjuif mentioned this pull request Feb 12, 2018

Split packages unirakun/k-ramel#69

Closed

migrate talks saga to listeners

85a9612

fabienjuif reviewed Feb 13, 2018

View reviewed changes

bpetetot added 4 commits February 13, 2018 16:56

migrate events saga to listeners

bc71430

use of store.dispatch('action') shortcut

cf26493

migrate submission saga to listeners

ed45cf2

migrate proposals and ratings sagas in listeners

42d1675

bpetetot added 2 commits February 13, 2018 20:10

fix bug of profile form

49c9050

add event loading on proposal route

7c998c7

fabienjuif mentioned this pull request Feb 14, 2018

listen hoc should dispatch a "PLUGUED" action unirakun/k-ramel#75

Closed

refactor forms using redux-form drivers

b77e79b

fabienjuif reviewed Feb 14, 2018

View reviewed changes

update dependencies

e219452

bpetetot mentioned this pull request Feb 17, 2018

better management of data fetch when mounting / routing #135

Closed

bpetetot merged commit 6482847 into master Feb 17, 2018

bpetetot deleted the 104-migrate-to-k-ramel branch February 17, 2018 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ migrate to k-ramel #123

✨ migrate to k-ramel #123

bpetetot commented Feb 10, 2018 •

edited

fabienjuif commented Feb 11, 2018

bpetetot commented Feb 11, 2018 •

edited

fabienjuif Feb 12, 2018 •

edited

bpetetot Feb 12, 2018

fabienjuif Feb 12, 2018

fabienjuif Feb 13, 2018

bpetetot Feb 13, 2018

bpetetot commented Feb 13, 2018

guillaumecrespel commented Feb 14, 2018

fabienjuif commented Feb 14, 2018

bpetetot commented Feb 14, 2018 •

edited

fabienjuif commented Feb 14, 2018

bpetetot commented Feb 14, 2018

fabienjuif Feb 14, 2018

fabienjuif Feb 14, 2018

bpetetot Feb 14, 2018

fabienjuif Feb 14, 2018

fabienjuif Feb 14, 2018

bpetetot Feb 14, 2018

fabienjuif Feb 14, 2018

bpetetot Feb 14, 2018

fabienjuif commented Feb 17, 2018

		@@ -1,19 +1,20 @@
		import { connect } from 'react-redux'
		import { inject } from 'k-ramel/react'

✨ migrate to k-ramel #123

✨ migrate to k-ramel #123

Conversation

bpetetot commented Feb 10, 2018 • edited

fabienjuif commented Feb 11, 2018

bpetetot commented Feb 11, 2018 • edited

fabienjuif Feb 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpetetot commented Feb 13, 2018

guillaumecrespel commented Feb 14, 2018

fabienjuif commented Feb 14, 2018

bpetetot commented Feb 14, 2018 • edited

fabienjuif commented Feb 14, 2018

bpetetot commented Feb 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabienjuif commented Feb 17, 2018

bpetetot commented Feb 10, 2018 •

edited

bpetetot commented Feb 11, 2018 •

edited

fabienjuif Feb 12, 2018 •

edited

bpetetot commented Feb 14, 2018 •

edited