Prepare RNW for DOM specific ResponderEventPlugin #908
Conversation
One thing I found out while working on necolas#908 this is that the 2048 game demo does not work on browsers since it listens for touch events only. This can be solved by listening to both mouse and touch events.
philipp-spiess
force-pushed
the
dom-responder
branch
from
5f90627
to
1a85a51
Compare
|
While working on this, did you notice that upgrading to React(DOM) 16.3 breaks the storybook? After I upgrade the project to use React 16.3 instead of 16.2, none of the touchables work and I see these warnings in the console: |
|
I'm super confused now. I was just testing my branch again (which is based on a commit that was created after the 16.3.2 commit) by placing the build artifacts into I then updated to 16.3.2 stable (via yarn) and saw the issue you described. Strangely I can't get my version to run anymore now. 🤔 |
|
OK it looks like maybe something unexpected changed in React or React DOM between 16.2 and 16.3 |
|
Ok I think I had different versions for the |
|
Just brainstorming here but can it be that after the upgrade, the website (= storybook) runs on a different react-dom than the one we inject the responderplugin in? |
|
Possibly. I think the benchmark's "Run" button was also broken after the upgrade though |
I've made my attempt at upgrading RNW to React 16.3.2 and it seems like everything is still working including the website and the benchmarks. I'm pretty sure that some obscure dependency graph caused Yarn to not de-dupe React in my first try as part of necolas#908. To fix this, I forced an update of all transitive dependencies as well so I suggest you also look over the updated `yarn.lock` in more detail since there where quite a few packages with minor updates. `yarn check` still complains about an unmet dependency: ``` β react-native-web git:(react-16.3.2) ✗ yarn check yarn check v1.3.2 warning "chokidar#fsevents#node-pre-gyp@^0.9.0" could be deduped from "0.9.1" to "[email protected]" error "benchmarks#reactxp#[email protected]" doesn't satisfy found match of "[email protected]" error "benchmarks#reactxp#[email protected]" doesn't satisfy found match of "[email protected]" warning "styled-jsx#stylis-rule-sheet#[email protected]" could be deduped from "3.5.0" to "[email protected]" info Found 2 warnings. error Found 2 errors. info Visit https://yarnpkg.com/en/docs/cli/check for documentation about this command. ``` I tried upgrading `reactxp` as well but that broke the benchmark script. I think this error can be ignored for now as the current version of `reactxp` still works fine with React 16.3.2. I've collected a couple of benchmarks before upgrading all dependencies and compared those after upgrading. I can't spot differences that are out of standard deviation but I'm pasting them here anyhow so you can verify. ## Before  ## After 
philipp-spiess
force-pushed
the
dom-responder
branch
from
2293166
to
d795275
Compare
# Conflicts: # packages/react-native-web/src/modules/ResponderEventPlugin/index.js
|
I am running into this issue now too. What was strange is that in my package.json I had react dependencies set to "^16.3.2" and it was working, working, working. Untill, yesterday when I needed to refresh my node_modules. @philipp-spiess , may be that's why you did not observe a problem, and then after you reinstalled your node_modules - it showed up. I cannot go back to 16.2 (and I cannot go forward to RNW 7.2+ (as my vector icons do not work in 7.2), so will stay at RN "16.3.2" RNW 0.6.1 for the time being. P.S. Another strange thing I noticed with this accidental React 16.4 pull -- is that the checkboxes are no longer styled by RNW, instead they look like the 'browser's default checkbox. So not only the touch events are not working there. |
|
I’m sorry this broke things for you. React doesn't follow semver when making changes to unofficial APIs, like the ones exposed for React Native for Web. |
|
@necolas , thank you. Understood. |
|
I was working on 0.8.0 when React 16.4 came out, but will pause that to make a patch release for React 16.4 and 0.7.x today. |
|
Thanks a lot for the help here. This fix is in 0.7.3 |
React 16.4 includes the necessary ResponderEventPlugin dependencies and makes some changes to the event internals that causes a breaking change. This patch fixes rendering with [email protected] while preserving backward-compatibility with earlier versions of react-dom. Close necolas#908
This PR prepares RNW for #12629. To keep changes at a minimum, the added dependencies were upstreamed to
react-dom.State of this PR
This PR is currently blocked by #12629 and will need a new release of
react-domafter #12629 is merged to function properly. I've verified that everything still works by linking against a local build ofreactandreact-domand running the examples viayarn website.