Skip to content
This repository has been archived by the owner on Jul 19, 2019. It is now read-only.

Require reconciler code from ReactDOM #104

Closed
wants to merge 1 commit into from

Conversation

dsblv
Copy link

@dsblv dsblv commented Oct 21, 2016

As pointed in facebook/react#7770 (comment), some internal modules of React 15.4.0 will be moved to react-dom package. When this happens, React ART will have to require ReactInstanceMap, ReactMultiChild, ReactUpdates from react-dom instead of react.

Fixes #103.

Note for maintainers: Before merging this, you might want to revert db4dcef, since it's purpose is delayed until React 16 (facebook/react#7598). This is the reason why tests are failing right now.

💃

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@quinn
Copy link

quinn commented Nov 22, 2016

seems like this should be merged / released to npm?

@dsblv
Copy link
Author

dsblv commented Nov 22, 2016

There must be a problem with @facebook-github-bot , because I did sign that CLA. Anyway, I won't be upset if someone else makes the change, let's make it happen!

@quinn
Copy link

quinn commented Nov 22, 2016

i also had to change this line:

const ContainerMixin = assign({}, ReactMultiChild, {

to this:

const ContainerMixin = assign({}, ReactMultiChild.Mixin, {

maybe that is a difference in a newer version of react or something. I ended up just cloning the repo and poking it until it started working : p idk anything about react internals

@dsblv
Copy link
Author

dsblv commented Nov 22, 2016

Yeah, I have a note about that in my first comment

@quinn
Copy link

quinn commented Nov 22, 2016

ah yeah missed that :)

@robhogan
Copy link

Until this is merged, it's easy enough to patch react-art with a postinstall script in your project's package.json. For macOS and linux:

"postinstall": "sed -i -e 's:react/lib:react-dom/lib:g' node_modules/react-art/lib/ReactART.js"

Azerothian pushed a commit to RungeICT/react-art that referenced this pull request Dec 5, 2016
@Xuewa
Copy link

Xuewa commented Dec 6, 2016

Thanks @quinn .I have solved my problem.

@dsblv
Copy link
Author

dsblv commented Dec 14, 2016

Well cool, it's done and in master now 🎉

@dsblv dsblv closed this Dec 14, 2016
@sophiebits
Copy link
Member

Sorry we didn't get this merged. Thanks for sending!

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

Successfully merging this pull request may close these issues.

6 participants