Skip to content
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

React v16.3 #165

Open
roman01la opened this issue May 8, 2018 · 14 comments
Open

React v16.3 #165

roman01la opened this issue May 8, 2018 · 14 comments

Comments

@roman01la
Copy link
Collaborator

roman01la commented May 8, 2018

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

@DjebbZ
Copy link
Contributor

DjebbZ commented Jun 1, 2018

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

@maximgatilin
Copy link

maximgatilin commented Jul 1, 2018

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

  1. componentWillReceiveProps
  2. componentWillMount
  3. componentWillUpdate

@roman01la
Copy link
Collaborator Author

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

@maximgatilin
Copy link

@roman01la Got it

@roman01la
Copy link
Collaborator Author

@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 👍

@DjebbZ
Copy link
Contributor

DjebbZ commented Sep 5, 2018

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
Copy link
Contributor

DjebbZ commented Sep 5, 2018

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

Not in Clojure JVM side.

@shvets-sergey
Copy link

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!

@tonsky
Copy link
Owner

tonsky commented Jun 5, 2019

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

@DavidAlphaFox
Copy link

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.

@tonsky
Copy link
Owner

tonsky commented Feb 9, 2023

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

@serioga
Copy link

serioga commented Feb 9, 2023

There is no equivalent replacement for Rum :-(

@DavidAlphaFox
Copy link

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

@tonsky
Copy link
Owner

tonsky commented Feb 10, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants