ckirkendall / kioo

Enlive/Enfocus style templating for Facebook's React and Om in ClojureScript.
Eclipse Public License 1.0
404 stars 39 forks source link

React-16: Supported Version #75

Open bteepell opened 6 years ago

bteepell commented 6 years ago

Do you have any plans to upgrade supported React version?

njordhov commented 6 years ago

It would be a step forward if kioo.util/WrapComponent didn't call the no longer supported React.createClass at loading time but rather delayed the call until first use by using e.g. memoize. That way, we may use create-react-class for a smoother transition to React-16 even before kioo is fully upgraded.

ckirkendall commented 6 years ago

I do plan on supporting react 16. I need to spend some time thinking about the memoize idea but I like the idea.

njordhov commented 6 years ago

The proposal enables releasing an interim version of kioo that uses React 15 yet allows developers to override with React 16. The changes are minor and should have ignorable impact on performance, only postponing the React calls from loading time to once at runtime. In brief, the current WrapComponent may be changed to:

(defn WrapComponent [props]                                                                                     
  ((wrap-component-fn) props))

where wrap-component-fn is a memoized fn contaning the current WrapComponent code:

 (def wrap-component-fn
   (memoize 
     (fn []
       (.createFactory js/React  
         (.createClass js/React 
         ....                                                                                 
njordhov commented 6 years ago

Also update dependencies to [sablono "0.8.1"] or later as 0.8.0 still calls React.createClass at loading time.

jocrau commented 6 years ago

I am not sure if memoizing the factory is necessary here. WarpComponent is called in handle-wrapper with a props argument that is a JavaScript object. Memoization stores the value the function evaluates to in a map using the (sequence of) arguments it was called with as key. But the subsequent props are never identical or even equal to any props stored in the memoization map, because (= (clj->js {:foo 1}) (clj->js {:foo 1})) => false.

jocrau commented 6 years ago

There is currently a mix of incompatible libraries and it is difficult to find the right way to upgrade. The dimensions of complexity are:

Sablono 0.8.4 currently goes the route of [cljsjs, v16, inherit]. For kioo in the short term the combination of [cljsjs, v16, create-react-class] might be the best. In the long run [npm, v16, inheritance] seems to be the future - at least to me. What are your thoughts?

njordhov commented 6 years ago

@jocrau Note that the proposed memoization is on a function with zero arguments returning a function that takes props as argument. Hence the memoized function is called just once, with the function returned being called with the different props. No props are stored in the memoization map.

jocrau commented 6 years ago

@TerjeNorderhaug You are right. Memoization will cause React.createFactory to only be called once.

I will try to update Kioo to use create-react-class and send a pull request.

ckirkendall commented 6 years ago

@TerjeNorderhaug That would be awesome!

rastandy commented 4 years ago

Same problem here. Would love to see this fixed and continue to use this great library.