fix: web compatibility #406

jaulz · 2019-01-09T10:46:12Z

The motivation for me was to use react-navigation@^3 and I stumbled across quite a few missing imports. This PR will introduce some shims so it shouldn't be a problem anymore.

osdnk · 2019-01-09T15:06:22Z

@jaulz
Hi, thanks for you PR. I believe it's a crucial step for RNGH!
Is it WIP? If yes, please add proper label?

jaulz · 2019-01-09T15:32:46Z

@osdnk sorry, I found some issues with the original commit and pushed a fix now 😊As requested I added some comments and it should be in a state where you could please take a look. I tried to touch the existing files at a minimum and also cross checked on native devices if it still works as expected.

osdnk

I haven't digged in this PR yet. Now leaving one comment, but it's not crucial.

osdnk · 2019-01-09T17:14:33Z

GestureHandler.web.js

+import gestureHandlerRootHOC from './gestureHandlerRootHOC';
+
+const State = {
+  UNDETERMINED: 'UNDETERMINED',


Can we move it out somewhere?

What should we move out in detail? State constants?

Yes. If you need them here, I wish to have them in separated file in order not to duplicate code.

Sure, makes more than sense 😊 Was not sure before since these constants were used in GestureHandler.js as well.

Oh, well, we intentionally wanted to take them from native site every time. How I think there's no need to do it. You could think about removing extracting from from Java and Objective-C if it's not big deal for you.

Hm, the constants are used in the native part as well. Hence it actually makes sense to read them in the JavaScript part from the native side. Should I undo the changes in GestureHandler.js for the time being?

No, it's fine now

necolas · 2019-01-10T19:02:12Z

AnimatedEvent.web.js

@@ -0,0 +1,3 @@
+import AnimatedEvent from 'react-native-web/dist/vendor/react-native/Animated/AnimatedEvent';


If AnimatedEvent needs to be used directly, you may want to ask that it is added to the Animated export from React Native rather than relying on the fragility of reaching into internals.

Okay, I will check on their repository and raise a PR.

Will keep track here: facebook/react-native#22984

necolas · 2019-01-10T19:02:49Z

GestureHandler.web.js

+  ToolbarAndroid,
+  ViewPagerAndroid,
+  DrawerLayoutAndroid,
+  WebView,


This module is being removed from React Native and isn't included in react-native-web either

Which one in detail? Currently, I see that all of these components are exported though some of them as UnimplementedView which is fine I guess.

react-native-community/discussions-and-proposals#3

Thanks for the info!

necolas · 2019-01-10T19:04:56Z

GestureHandler.web.js

+    }
+
+    setNativeProps() {
+      // No implementation so far but is needed to avoid null calls


You should be able to use this.container.current.setNativeProps

I intentionally left this out since I think we do not need to pass on props for stub components. This would just generate warnings since most of the props are custom.

necolas · 2019-01-10T19:05:48Z

GestureHandler.web.js

+        x,
+      },
+    });
+  node.addEventListener('click', clickHandler);


Using these DOM events directly means you're not hooked into the responder event system

Do you have any link for me to see how we could improve this?

I'm not familiar with the behaviour/implementation of the native TapGestureHandler. But if it turns out to be preferable to use the responder events, you could pass the event handlers to createHandler and forward them to the component.

necolas · 2019-01-10T19:06:47Z

GestureHandler.web.js

+    static displayName = name;
+
+    render() {
+      return <TouchableWithoutFeedback {...this.props} />;


If these are meant to be used as buttons in the UI, you should add accessibilityRole="button"

Thanks for the hint!

miracle2k · 2019-01-12T14:08:22Z

Sorry for the interjection:

I was trying this PR, but the thing I run into is that my babel transforms the access to this in the static property in this line (https://github.com/kmagiera/react-native-gesture-handler/blob/master/touchables/GenericTouchable.js#L56) into

GenericTouchable2.propTypes = _objectSpread({}, (void 0).internalPropTypes, (void 0).publicPropTypes);

(i.e., it replaces this with undefined).

Clearly something is wrong with the way I have setup my babel for the react-native-web compile. Can @jaulz or someone else share a working babel config?

necolas · 2019-01-12T15:49:20Z

That just looks like incorrect code. this should be the class name instead

miracle2k · 2019-01-12T20:51:05Z

@necolas Funny. If I change those lines to

static propTypes = {
    ... GenericTouchable.internalPropTypes,
    ... GenericTouchable.publicPropTypes,
  };

It compiles with webpack, but then react-native@0.57.8 in the iOS emulator says:

Cannot read property 'internalPropTypes' of undefined

The exact opposite behaviour.

miracle2k · 2019-01-12T21:51:33Z

I am fairly certain this is related to the Hot Module Reloading transform inserted by metro here by merging into babelrc this partial config.

Transforming the file in question with babel and using only metro-react-native-babel-preset, the output is sensible: https://gist.github.com/miracle2k/3bc7f1c50080397dbf0d92cfb3101677#file-generictouchable-babel-with-metro-preset-js.

But the actual code generated by react-native for the iOS emulator looks like this: https://gist.github.com/miracle2k/3bc7f1c50080397dbf0d92cfb3101677#file-generictouchable-rn-compilation-js
Notice the _wrapComponent calls which come from the react-transform plugin: https://github.com/facebook/metro/blob/master/packages/metro-babel7-plugin-react-transform/src/index.js#L420.

The problem goes away if I manually edit the file linked above (reactNativeTransformer.js#L131) to ensure the transform is not added. Then, like @necolas says, using the classname instead of this works. Note that the current react-native ships with an old metro version, v0.48.5, so the problem may or may not be fixed in a newer one.

I can see that @osdnk added this code quite recently. Maybe as a workaround for this issue we could rewrite that part and pull the re-used proptypes into the global module scope.

osdnk · 2019-01-12T22:06:48Z

I will merge it if needed.

fix: web compatibility

Fixes the issue we encountered #406 (comment) To review, the problem is as follows: The correct syntax is: ``` static propTypes = { ...GenericTouchable.internalPropTypes } ``` Metro/Babel compiles this to: ``` // Define GenericTouchable GenericTouchable.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes); ``` But when metro then runs the HMR transform, this becomes: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes), ``` It rewrites the class name to `_class` during initialization, but misses the references in the spread, which then causes a undefined-access exception. See this Gist for full outputs: https://gist.github.com/miracle2k/3bc7f1c50080397dbf0d92cfb3101677#file-generictouchable-babel-with-metro-preset-js Part of the problem seems to be that metro uses a very old version of [react-hot-loader](facebook/metro#336). The code as it is currently in the library, using `...this.publicPropTypes` has two problems: First, it does not compile at all with babel, when this library becomes imported as part of using `react-native-web`. Second, while it does compile using metro, the generated code instead will be: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, this.internalPropTypes, this.publicPropTypes), ``` With `this` probably referring to the window or global scope, thus not causing a crash, but not actually setting the proptypes correctly.

jaulz · 2019-01-14T14:32:20Z

@osdnk @brentvatne it would be great if could you have another look. I assume we could even implement a proper PanGestureHandler with the help of PanResponder from react-native-web but this might be something for the future.

DrawerLayout.js

GestureHandler.web.js

axhamre · 2019-02-19T12:05:01Z

May I ask what's the status here?

jaulz · 2019-02-19T12:37:22Z

@kmagiera could you please check the latest version?

iamnader · 2019-02-22T15:11:43Z

excited for this. thanks for all the work here folks

kmagiera

Thanks @jaulz so much for working on this. This PR is excellent 👌

I left one small comment, however I'm going to merge this anyways. If you would like to make respond to that comment please submit a separate PR.

kmagiera · 2019-02-25T13:19:42Z

createHandler.web.js

+    };
+
+    render() {
+      let Handler = handlers[handlerName];


nit: I think this check could be moved outside of the render method and directly to createHandler

jaulz · 2019-02-27T16:43:48Z

@kmagiera do you have any timeline for a new release? Thanks for merging by the way 😊

kmagiera · 2019-02-28T18:44:22Z

will try to send out new release tomorrow

alidcast · 2019-03-04T15:05:25Z

just tried using release with this PR for web and ran into errors. what do we need to configure to get it running - looks like index.web needs to be resolved over index, anything else?

it'd be great we we could just have a simple alias (like we do react-native to react-native-web)

jaulz · 2019-03-04T15:16:38Z

@alidcastano if you add the .web.js suffix to your Webpack configuration (in resolve.extensions) it should work immediately.

Summary: Initially if a `react-native-web` project were to use a library that required internals `expo/webpack-config` would polyfill those internals. This `Platform.web` was used for cases where `react-native` modules needed other internal `react-native` modules, ex: `Animated/src/AnimatedEvent -> Renderer/shims/ReactNative -> Renderer/oss/ReactNativeRenderer-dev -> Core/InitializeCore -> Devtools/setupDevtools -> WebSocket/WebSocket -> Utilities/Platform` The consensus is that if any `react-native` library references a `react-native` internal (ex: `react-native/*`), it should continue to throw errors. We've removed the use of internals from all of the Unimodules, `react-navigation`, and `react-native-gesture-handler`. This covers a wide enough area for a lot of projects to get web support. * Add emitters for libs referencing internals necolas/react-native-web#1275 * Remove monkey patch that bundles RN, libs that use internals will crash instead expo/expo-cli#409 * Remove all unsupported internals and polyfills from the Expo suite expo/expo#3676 * Remove internals from react-native-gesture-handler software-mansion/react-native-gesture-handler#406 * Related #23387 [GENERAL] [REMOVED] - Platform.web.js Pull Request resolved: #23830 Differential Revision: D14406145 Pulled By: hramos fbshipit-source-id: bdda99a334d33f5543fdb954eb80e2e7186f985a

osdnk · 2019-05-13T12:34:09Z

createHandler.js

+      RNGestureHandlerModule.updateGestureHandler(this._handlerTag, newConfig);
+    };
+
+    _dropGestureHandler = () => {


Why is it not used?

What do you mean?

_dropGestureHandler is not used anywhere. Isn't it a bug?

Yep, good catch! Sorry..

np, thanks!

/pull/406/files#diff-88fa64c878efe14d137f130ddbe000a5R185 From unknown reason, it has been changed here. Probably it's just a mistake, but it was leading to memory leaks.

orestis-z · 2019-06-07T12:39:07Z

I opened an issue for two encountered bugs here: #632
Please have a look.

…-mansion#412) Fixes the issue we encountered software-mansion#406 (comment) To review, the problem is as follows: The correct syntax is: ``` static propTypes = { ...GenericTouchable.internalPropTypes } ``` Metro/Babel compiles this to: ``` // Define GenericTouchable GenericTouchable.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes); ``` But when metro then runs the HMR transform, this becomes: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, GenericTouchable.internalPropTypes, GenericTouchable.publicPropTypes), ``` It rewrites the class name to `_class` during initialization, but misses the references in the spread, which then causes a undefined-access exception. See this Gist for full outputs: https://gist.github.com/miracle2k/3bc7f1c50080397dbf0d92cfb3101677#file-generictouchable-babel-with-metro-preset-js Part of the problem seems to be that metro uses a very old version of [react-hot-loader](facebook/metro#336). The code as it is currently in the library, using `...this.publicPropTypes` has two problems: First, it does not compile at all with babel, when this library becomes imported as part of using `react-native-web`. Second, while it does compile using metro, the generated code instead will be: ``` // Create _class _class.propTypes = (0, _objectSpread2.default)({}, this.internalPropTypes, this.publicPropTypes), ``` With `this` probably referring to the window or global scope, thus not causing a crash, but not actually setting the proptypes correctly.

The motivation for me was to use `react-navigation@^3` and I stumbled across quite a few missing imports. This PR will introduce some shims so it shouldn't be a problem anymore.

software-mansion/pull/406/files#diff-88fa64c878efe14d137f130ddbe000a5R185 From unknown reason, it has been changed here. Probably it's just a mistake, but it was leading to memory leaks.

Manishalexin · 2020-04-05T19:44:51Z

When trying to import from RNGH in react native web project, it throws with this error,
"Support for the experimental syntax 'exportDefaultFrom' isn't currently enabled", adding babel plugins for these doesn't help.

Minimum reproducible demo,
https://codesandbox.io/s/polished-paper-tdmyp

brentvatne · 2020-04-05T19:55:23Z

@Manishalexin - you should use babel-preset-expo and the react-native-web setup that you get when you init a project with expo-cli - npm i -g expo-cli && expo init, or follow this guide to add to an existing project: https://docs.expo.io/versions/latest/guides/running-in-the-browser/

here's an example of it running in snack: https://snack.expo.io/@notbrent/jealous-coffee

Manishalexin · 2020-04-05T22:17:04Z

Thank you @brentvatne it's working with babel-preset-expo in babel config and I have to use expo start. React scripts & react-scripts-rewired still fails. So, now the storybook fails as it still uses react scripts. I used the setup here, https://github.com/expo/examples/tree/master/with-storybook but it doesn't work as I believe it's only for the expo projects. Any suggestions on how to make it work?

I am using a bare workflow, with react native cli.

jpaas · 2020-05-19T12:24:20Z

Also running into this on a create-react-app project using RNGH 1.6.1

SyntaxError: .../node_modules/react-native-gesture-handler/DrawerLayout.js: Support for the experimental syntax 'flow' isn't currently enabled (30:8):

  28 | const SETTLING = 'Settling';
  29 |
> 30 | export type PropType = {
     |        ^
  31 |   children: any,
  32 |   drawerBackgroundColor?: string,
  33 |   drawerPosition: 'left' | 'right',

afilp · 2020-08-21T17:34:22Z

I do not use expo, how can we solve the dreaded /react-native-gesture-handler/DrawerLayout.js: Support for the experimental syntax 'exportDefaultFrom' isn't currently enabled (30:8): issue?
I tried using react-app-rewired but this does not seem to work, I get other errors and I am not sure if we should go this way.

Thanks a lot!

I also tried the expo solution (although the app is not an expo app), but now I get this weird error:

 web  Failed to compile.
D:/R/myApp/node_modules/react-native/Libraries/Network/XMLHttpRequest.js
Module not found: Can't resolve './RCTNetworking' in 'D:\R\myApp\node_modules\react-native\Li
braries\Network'
Error from chokidar (D:\): Error: EBUSY: resource busy or locked, lstat 'D:\pagefile.sys'

EDIT: This was related to reactotron, so not an issue, see here for solution:
https://forums.expo.io/t/failed-to-compile-cant-resolve-rctnetworking/26273/7

kopax-polyconseil · 2021-06-28T11:50:00Z

Hi, I am trying to add react-native-web support on an existing react native ios android application,

I still have error such as this one :

 FAIL  src/features/search/pages/__tests__/SearchFilter.test.tsx
  ● Test suite failed to run

    TypeError: (0 , _reactNative.requireNativeComponent) is not a function

      1 | import { t } from '@lingui/macro'
      2 | import React, { useState } from 'react'
    > 3 | import { TouchableOpacity } from 'react-native-gesture-handler'
        | ^
      4 | import styled from 'styled-components/native'
      5 | 
      6 | import { DateFilter } from 'features/search/atoms/Buttons'

      at Object.<anonymous> (node_modules/react-native-gesture-handler/GestureHandlerButton.js:3:32)
      at Object.<anonymous> (node_modules/react-native-gesture-handler/GestureButtons.js:6:1)
      at Object.<anonymous> (node_modules/react-native-gesture-handler/GestureHandler.js:10:1)
      at Object.<anonymous> (node_modules/react-native-gesture-handler/Swipeable.js:10:1)
      at Object.<anonymous> (node_modules/react-native-gesture-handler/index.js:1:1)
      at Object.<anonymous> (src/features/search/sections/OfferDate.tsx:3:1)
      at Object.<anonymous> (src/features/search/sections/index.ts:8:1)
      at Object.<anonymous> (src/features/search/pages/SearchFilter.tsx:10:1)
      at Object.<anonymous> (src/features/search/pages/__tests__/SearchFilter.test.tsx:12:1)

Looking at your discussion, I assume it should work out of the box with react-native-gesture-handler v1.6.0, why am I still getting those requireNative? Thanks a lot for this awsome lib!

fix: web compatibility

94670f3

jaulz mentioned this pull request Jan 9, 2019

Web support #243

Closed

fix: implement TapGestureHandler

96c8515

fix: treat enabled property as true if not set explicitely

3867bfa

osdnk reviewed Jan 9, 2019

View reviewed changes

Julian Hundeloh and others added 2 commits January 10, 2019 10:45

fix: move Directions and State to separate modules

a9cd8fb

fix: add dummy setNativeProps

97236fd

gut4 mentioned this pull request Jan 10, 2019

React native web crashes with react-navigation necolas/react-native-web#1224

Closed

necolas reviewed Jan 10, 2019

View reviewed changes

This was referenced Jan 13, 2019

Remove HMR feature facebook/metro#336

Open

Workaround metro not compiling static properties correctly. #412

Merged

miracle2k added a commit to miracle2k/react-native-gesture-handler that referenced this pull request Jan 13, 2019

Merge pull request software-mansion#406 from jaulz/fix/web-compatibility

f9ae30b

fix: web compatibility

jaulz changed the title ~~fix: web compatibility~~ [WIP] fix: web compatibility Jan 14, 2019

fix: use responder events

ee685df

jaulz changed the title ~~[WIP] fix: web compatibility~~ fix: web compatibility Jan 14, 2019

osdnk reviewed Jan 14, 2019

View reviewed changes

DrawerLayout.js Outdated Show resolved Hide resolved

osdnk reviewed Jan 15, 2019

View reviewed changes

GestureHandler.web.js Outdated Show resolved Hide resolved

osdnk reviewed Jan 15, 2019

View reviewed changes

GestureHandler.web.js Outdated Show resolved Hide resolved

osdnk reviewed Jan 15, 2019

View reviewed changes

GestureHandler.web.js Outdated Show resolved Hide resolved

osdnk reviewed Jan 15, 2019

View reviewed changes

GestureHandler.web.js Outdated Show resolved Hide resolved

fix: map Responder event properties

cd9bc75

fix: add createHandler file

4f568bc

EvanBacon mentioned this pull request Feb 22, 2019

[Web] More web support react-navigation/stack#88

Merged

kmagiera approved these changes Feb 25, 2019

View reviewed changes

kmagiera merged commit c9f7c46 into software-mansion:master Feb 25, 2019

jaulz deleted the fix/web-compatibility branch February 26, 2019 10:03

jclough96 mentioned this pull request Feb 27, 2019

Upgrade dependencies for react-native-gesture-handler to 1.0.16 to support react-native-web react-navigation/react-navigation#5632

Closed

jaulz restored the fix/web-compatibility branch February 27, 2019 16:43

blackpencil8994 mentioned this pull request Mar 4, 2019

null is not an object ( evaluating 'RNGestureHandlerModule.Direction') #494

Closed

EvanBacon mentioned this pull request Mar 9, 2019

Removed Platform.web facebook/react-native#23830

Closed

osdnk reviewed May 13, 2019

View reviewed changes

osdnk added a commit that referenced this pull request May 13, 2019

Restore dropping handler or unmounting (#604)

2f45289

/pull/406/files#diff-88fa64c878efe14d137f130ddbe000a5R185 From unknown reason, it has been changed here. Probably it's just a mistake, but it was leading to memory leaks.

jaulz deleted the fix/web-compatibility branch September 3, 2020 05:56

		@@ -0,0 +1,3 @@
		import AnimatedEvent from 'react-native-web/dist/vendor/react-native/Animated/AnimatedEvent';

fix: web compatibility #406

fix: web compatibility #406

Conversation

jaulz commented Jan 9, 2019

osdnk commented Jan 9, 2019 • edited Loading

jaulz commented Jan 9, 2019

osdnk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osdnk Jan 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miracle2k commented Jan 12, 2019

necolas commented Jan 12, 2019

miracle2k commented Jan 12, 2019

miracle2k commented Jan 12, 2019 • edited Loading

osdnk commented Jan 12, 2019

jaulz commented Jan 14, 2019

axhamre commented Feb 19, 2019 • edited Loading

jaulz commented Feb 19, 2019

iamnader commented Feb 22, 2019

kmagiera left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaulz commented Feb 27, 2019

kmagiera commented Feb 28, 2019

alidcast commented Mar 4, 2019 • edited Loading

jaulz commented Mar 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orestis-z commented Jun 7, 2019 • edited Loading

Manishalexin commented Apr 5, 2020

brentvatne commented Apr 5, 2020 • edited Loading

Manishalexin commented Apr 5, 2020

jpaas commented May 19, 2020

afilp commented Aug 21, 2020 • edited Loading

kopax-polyconseil commented Jun 28, 2021

osdnk commented Jan 9, 2019 •

edited

Loading

osdnk Jan 9, 2019 •

edited

Loading

miracle2k commented Jan 12, 2019 •

edited

Loading

axhamre commented Feb 19, 2019 •

edited

Loading

kmagiera left a comment •

edited

Loading

alidcast commented Mar 4, 2019 •

edited

Loading

orestis-z commented Jun 7, 2019 •

edited

Loading

brentvatne commented Apr 5, 2020 •

edited

Loading

afilp commented Aug 21, 2020 •

edited

Loading