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

Upgrade to [email protected] #255

Closed
wants to merge 1 commit into from
Closed

Conversation

MoOx
Copy link
Contributor

@MoOx MoOx commented Nov 17, 2016

This patch solves the following problem

I tried to update react to latest 15.4.0 to be able to use jest without this blocking issue but it seems RNW use some /lib/* stuff.

Test plan

I just upgraded all reference to react-dom. With react 15.4.0, some lib/* redundant in both react and react-dom packages but I think it's better to rely on a single package so I just switched all references to react/lib/ to react-dom/lib/ and it worked.

This pull request

  • includes documentation
  • includes tests
  • includes an interactive example
  • includes screenshots/videos

Note: Tests were breaking before this update, with the changes included in this PR they pass.

@necolas
Copy link
Owner

necolas commented Nov 21, 2016

I tried this locally and found that the touchables stopped working in the examples.

@necolas necolas mentioned this pull request Nov 21, 2016
@necolas
Copy link
Owner

necolas commented Nov 21, 2016

The problem seems to be that the topLevelType in ResponderEventPlugin - used to be topMouseDown or topTouchStart but is now topClick. The isStartish helper now returns false (because it's only check for mouse/touch events) in 15.4 where it used to return true.

@necolas
Copy link
Owner

necolas commented Nov 23, 2016

Sigh. Because [email protected] is going to be a giant ball of pre-built code, we might need to update the usage instructions to suggest aliasing 'react' to 'react/lib/React'. I think this will let your bundler de-dupe all the modules shared between 'react' and 'react-dom'.

@remon-nashid
Copy link

remon-nashid commented Dec 9, 2016

Thanks for your effort to make the upgrade work. Upgrading is especially necessary because running jest tests while using react < 15.4 is currently impossible.

@necolas
Copy link
Owner

necolas commented Dec 9, 2016

IIRC, not impossible if you include jest.mock('react-dom'). I'm waiting for a patch to React to fix the ResponderEventPlugin

@remon-nashid
Copy link

remon-nashid commented Dec 10, 2016

I just realised that @MoOx has raised the issue originally for the same purpose. Thanks for the tip @necolas. Hopefully we get a feedback about facebook/react#8370 soon.

@nicholeuf
Copy link

Any idea when this will get merged?

@necolas
Copy link
Owner

necolas commented Dec 13, 2016

When facebook/react#8370 is resolved

@necolas
Copy link
Owner

necolas commented Dec 16, 2016

@MoOx would you mind rebasing and updating the branch. May make it easier to debug the (possible) React issue.

@MoOx
Copy link
Contributor Author

MoOx commented Dec 16, 2016

PR updated.

(There are some changes in the yarn.lock file, I guess it's normal since we changed some packages - I just "yarn"ed once before running the tests).

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.

4 participants