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

Use separate prop-types package for compatibility with react16+ #18

Merged
merged 1 commit into from
Jan 16, 2018

Conversation

chpill
Copy link
Contributor

@chpill chpill commented Jan 16, 2018

Using this library issues a warning regarding the fact that PropTypes will need to be accessed from a separate package starting with react16.

The version of the cljsjs/prop-types library used is the one recommended in the rum readme

@martinklepsch
Copy link
Owner

Thanks @chpill, that's a good catch. Is this backwards compatible with React 15?

@chpill
Copy link
Contributor Author

chpill commented Jan 16, 2018

It should be, the prop-types package is strictly equivalent to what was included with react until v16.

That said, I have only tested it with react16 (using some exclusions and this fork of rum )... I'll try to make use of this in other projects using react v15 to see if I see some breakage, but it is quite painful to test unreleased features of external libraries using lein or boot...

I'll try make use of the :local/root feature of the deps.edn used by the new clojure CLI and report back.

@chpill
Copy link
Contributor Author

chpill commented Jan 16, 2018

Just tested locally, works like a charm with react v15.4.2

@martinklepsch martinklepsch merged commit ddac01b into martinklepsch:master Jan 16, 2018
@martinklepsch
Copy link
Owner

@chpill I just cut a release: 0.3.0. It has a breaking change but since you already tested it with your change I assume you did not use derivatives-pool directly.

As usual details can be found in the changelog.

@chpill
Copy link
Contributor Author

chpill commented Jan 17, 2018

@martinklepsch Thanks a lot for the new release! I am in fact using derivatives-pool directly, but I was already using the latest version of the code (with the derivatives-pool record), so it's all good 👍

I have ditched using react context because i found it too cumbersome... In the end I use derivatives in combinations with citrus, i pass my reconciler around and keep the derivatives-pool in it.

I also made a mixin that registers and releases derivatives during :render, a bit like the rum.core/reactive mixin, so that I don't have to declare what derivatives I am going to use beforehand.

@martinklepsch
Copy link
Owner

martinklepsch commented Jan 17, 2018 via email

@chpill
Copy link
Contributor Author

chpill commented Jan 17, 2018

Sure, should I open a new issue to start the discussion?

@martinklepsch
Copy link
Owner

Feel free to go straight to PR. I think the usage section could be split in the following ways:

  • usage
    • generic (i.e. no specific react wrapper)
    • rum
    • citrus

If there are larger code blogs you could consider using <details></details> to hide them at first.

@chpill
Copy link
Contributor Author

chpill commented Jan 18, 2018

I'll try to contribute something soon. I have to clean up the way I initialize the whole thing before I can show it to other people 😅

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

Successfully merging this pull request may close these issues.

2 participants