rum icon indicating copy to clipboard operation
rum copied to clipboard

React v16.3

Open roman01la opened this issue 7 years ago • 14 comments

Tracking issue for React 16.3 API changes: https://reactjs.org/blog/2018/03/29/react-v-16-3.html

Let's discuss these changes before starting with implementation, cc @tonsky @piranha @DjebbZ @jetmind (mention more people)

~Official Context API~

Not sure if we need a wrapper at all, but here's an example.

Wrappers

(def create-context js/React.createContext)

(defn with-context [ctx render-child]
  (js/React.createElement (.-Consumer ctx) render-child))

(defn provide-context [ctx props & children]
  (apply js/React.createElement (.-Provider ctx) props children))

Usage

(def theme-ctx (rum/create-context "light")) ;; create Context instance

;; theme-aware component
(rum/defc Button [{:keys [theme]} text]
  [:button {:color (colors theme)} text])

;; context consuming wrapper
(rum/defc ThemedButton [text]
  (with-context theme-ctx #(Button {:theme %} text)))

;; usage
(rum/provide-context theme-ctx #js {:value "light"}
  (ThemedButton "Submit"))

~createRef API #204~

Wrapper

(def create-ref js/React.createRef)

Usage

(def jq-ref (rum/create-ref))

(rum/defc jqUIComponent <
  {:did-mount #(init-jq-plugin (.-current jq-ref))}
  []
  [:div {:ref jq-ref}])

or instance bound ref via userland mixin

(defn with-ref [key]
  {:init (fn [state _] (assoc state key (rum/create-ref)))})

(rum/defcs jqUIComponent <
  (with-ref :refs/jq)
  [{jq-ref :refs/jq}]
  [:div {:ref jq-ref}])

forwardRef API

Not sure if it will work with Rum. Needs investigation.

Component Lifecycle Changes

New lifecycle methods

getDerivedStateFromProps

getDerivedStateFromProps is being added as a safer alternative to the legacy componentWillReceiveProps. getDerivedStateFromProps is invoked after a component is instantiated as well as when it receives new props. It should return an object to update state, or null to indicate that the new props do not require any state updates.

Rum's :did-remount lifecycle mixin runs in componentWillReceiveProps, perhaps Rum could use getDerivedStateFromProps here (in non-breaking way).

getSnapshotBeforeUpdate

getSnapshotBeforeUpdate is being added to support safely reading properties from e.g. the DOM before updates are made. getSnapshotBeforeUpdate() is invoked right before the most recently rendered output is committed to e.g. the DOM. It enables your component to capture current values (e.g. scroll position) before they are potentially changed. Any value returned by this lifecycle will be passed as a parameter to componentDidUpdate().

Rum's :did-update and :after-render lifecycle mixins run in componentDidUpdate, perhaps Rum could use getSnapshotBeforeUpdate here (in non-breaking way).

~StrictMode Component~

I think we don't need a wrapper here, same as for React.Fragment

roman01la avatar May 08 '18 12:05 roman01la

Here's some quick comments:

Context: I think your example is wrong, because you're using with-context, which is supposed to be used to consume context, to provide a value for the themed button. I think you meant provide-context instead. Am I right ?

getDerivedStateFromProps:

perhaps Rum could use getDerivedStateFromProps here (in non-breaking way).

I think not. A new :derive-state lifecycle would be necessary IMO, which would correspond roughly to :init + :did-remount (but I'm confused about did-remount, never used it), since :did-remount isn't run before first render.

PS: I haven't seriously used nor played with React 16.3 new features yet

DjebbZ avatar Jun 01 '18 08:06 DjebbZ

I think you should also consider showing warnings for the deprecated lifecycle methods:

  1. componentWillReceiveProps
  2. componentWillMount
  3. componentWillUpdate

maximgatilin avatar Jul 01 '18 16:07 maximgatilin

@maximgatilin this is React’s job, it’ll warm anyway since Rum is using those methods

roman01la avatar Jul 02 '18 05:07 roman01la

@roman01la Got it

maximgatilin avatar Jul 02 '18 05:07 maximgatilin

@DjebbZ

I think you meant provide-context instead. Am I right ?

You are right, fixed.

I think not. A new :derive-state lifecycle would be necessary IMO...

Agreed, it would be better for compatibility to add new lifecycle hooks 👍

roman01la avatar Jul 02 '18 09:07 roman01la

Regarding forwardRef API, I've looked briefly at the code of React.js itself, it only sets a custom property on the internal React component object itself. My understanding of rum is that the only APIs that manipulate it are the rum/defc* macros, so maybe there should be a rum/defr (def with ref) ? And the combinations too, like rum/defcsr, rum/rdefccr etc.

Just sharing some thoughts, I did not try at all to implement it.

DjebbZ avatar Sep 05 '18 15:09 DjebbZ

this is React’s job, it’ll warm anyway since Rum is using those methods

Not in Clojure JVM side.

DjebbZ avatar Sep 05 '18 15:09 DjebbZ

Is there any intention to merge this pull request or update React to later versions at all? In my project, I have a couple of other libraries that depend on later versions of React and having two versions of React in the same project is a bad idea. I'm willing to spend time and update it (and prepare pull-request), but want to make sure that I'm not missing some reason on why rum needs to depend on the older version of React?

Thank you for the awesome library!

shvets-sergey avatar Jun 03 '19 14:06 shvets-sergey

@bearz you can use React 16.3 just fine, just override the version in the dependencies

tonsky avatar Jun 05 '19 11:06 tonsky

Dear owner and contributors I checked React 16.4, it's published nearly 5y ago. And the React lifecycle diagram seems stable after this release refer to the React lifecycle diagram. So could we make some break changes in rum to stop using the UNSAFE_'s APIs, or build a compatible layer based on getDerivedStateFromProps and getSnapshotBeforeUpdate? Acccording to You Probably Don't Need Derived State, I think we could simulate :will-mount, :will-update and maybe :will-remount with getDerivedStateFromProps and some controlling state in Rum. Also I think we could using :init instead of :will-mount.

DavidAlphaFox avatar Feb 09 '23 01:02 DavidAlphaFox

Rum is not actively maintained. Maybe choose another, more modern framework?

tonsky avatar Feb 09 '23 14:02 tonsky

There is no equivalent replacement for Rum :-(

serioga avatar Feb 09 '23 14:02 serioga

Dear owner I transfer from reagent to rum recently, I think rum is a high quality project to use React in clojurescript. Could we select a new leader to make this project into actively maintained status?

Thanks David Gao

DavidAlphaFox avatar Feb 10 '23 03:02 DavidAlphaFox

I agree that would be nice. Unfortunately my attempts did not succeeded so far

tonsky avatar Feb 10 '23 12:02 tonsky