civiccc / react-waypoint

A React component to execute a function whenever you scroll to an element.
MIT License
4.08k stars 208 forks source link

Make it possible to use react-waypoint in React Native #186

Open haruelrovix opened 7 years ago

haruelrovix commented 7 years ago

Quick question: Can I use react-waypoint for React Native project?

When I tried to import it, I got an error as in the title.

image

jamesplease commented 7 years ago

@haruelrovix , I've never used React Native, but I ran a few google searches to see if this is a common problem.

One possibly useful thread here: https://github.com/airbnb/react-native-maps/issues/1206

A recommendation people have would be for us to exclude the .babelrc file from the npm package. It shouldn't be needed for consumers, so that might be fine.

That doesn't help you much now, but there are a few workarounds in there that might help you get unblocked. I'd try some of those out – hopefully one of them works until we can figure something out on our end (if we end up needing to make changes)

@trotzig / @lencioni , what do you say? Should we try excluding the .babelrc, or do either of you have experience with React Native / other recommendations?

trotzig commented 7 years ago

That sound like a good start, although we'd have to make a few substantial changes to the component in order to remove the dependency on a DOM. Here are just a few places where we assume a DOM of some sort:

https://github.com/brigade/react-waypoint/blob/0ba33d8add28614c321fee97c81f4c94a136ce78/src/isDOMElement.js#L10

https://github.com/brigade/react-waypoint/blob/0ba33d8add28614c321fee97c81f4c94a136ce78/src/resolveScrollableAncestorProp.js#L5

https://github.com/brigade/react-waypoint/blob/0ba33d8add28614c321fee97c81f4c94a136ce78/src/waypoint.jsx#L226

I haven't done any react-native development, and we don't have anyone at my company doing it either. Perhaps @lencioni can reach out to someone to ask if it's a good idea/how to do it?

olegstepura commented 7 years ago

Webpack 2 is not happy with add-module-exports and fixing is easy https://gist.github.com/iamakulov/966b91c0fc6363a16ff0650b51fb991b

Can this dependency be removed?

olegstepura commented 7 years ago

I quickly took a look at your code and I wonder why do you need such dependency? Everything seem to be exported and imported in es6 style.

trotzig commented 7 years ago

This is tricky. I agree that it would be nice to get rid of the add-module-exports dependency. But doing so would break a lot of applications (because they would have to require('react-waypoint').default suddenly).

What version of webpack are you using? We're on 2.6 and I'm not seeing this issue.

I wonder if we could use js:next inside our package.json, and support both es6 projects (non-transpiled) and commonjs projects (transpiled with babel, including add-module-exports). Would that be something you'd be willing to explore?

olegstepura commented 7 years ago

As far as I understand this, webpack finds that .babel in react-waipoint and understands that add-module-exports is required. I'm using webpack 2.5.1 (just did not upgrade to 2.6 yet, they started to release too often.

the thing is that I'm using lot s of different libraries, like redux, redux-saga, react-toolbox, redux-form, store and still none of them required to add add-module-exports.

Not sure if js:next fixes the issue, but if does, it's worth trying

trotzig commented 7 years ago

Are you processing node_modules through babel by any chance? babel-plugin-add-module-exports is a dev dependency that we use in the build process. When you have installed react-waypoint, you shouldn't have to use babel to transpile it.

That being said, there might be something we could do to improve this. I noticed that redux has a js:next specified in its package.json, that could be a place to start.

olegstepura commented 7 years ago

Are you processing node_modules through babel by any chance?

Well, seems like that

  resolve: {
    extensions: ['.js', '.json', '.css'],
    modules: [
      'node_modules',
      'src/main/react', // allows importing modules using absolute path, @see https://goo.gl/luH0Xa
    ]
  },
  //...

        test: /\.js$/,
        loader: 'babel-loader',
trotzig commented 7 years ago

I see. For now I think you should exclude react-waypoint from being loaded by babel. Something like

  {
    test: /\.js$/,
    use: {
      loader: babelLoader,
    },
    exclude: [/node_modules\/react-waypoint/],
  },

This should work around your issue.

olegstepura commented 7 years ago

@trotzig Thanks!

Just tried this, did not help:

ERROR in ./~/react-waypoint/build/waypoint.js
Module build failed: ReferenceError: Unknown plugin "add-module-exports" specified in "[projectpath]\\node_modules\\react-waypoint\\.babelrc" at 1, attempted to resolve relative to "[projectpath]\\node_modules\\react-waypoint"
    at W:\mahjong-website\backend\node_modules\babel-core\lib\transformation\file\options\option-manager.js:180:17

Also tried (since I'm on windows):


        exclude: [
          /node_modules[\\\/]react-waypoint/
        ]

but this did not help as well, I faced this:

ERROR in ./~/consolidated-events/lib/index.js
Module build failed: Error: Couldn't find preset "airbnb" relative to directory "[projectpath]\\node_modules\\consolidated-events"
    at [projectpath]\node_modules\babel-core\lib\transformation\file\options\option-manager.js:293:19
    at Array.map (native)

So I had add that to excude as well:

        exclude: [
          /node_modules[\\\/]react-waypoint/,
          /node_modules[\\\/]consolidated-events/
        ],

and now it works!

trotzig commented 7 years ago

Glad you found a workaround!

olegstepura commented 7 years ago

Yep, but a plug-and-play solution should exists. Maybe it will be a js:next one. Feel free to ask me to check when npm version will be ready.

trotzig commented 7 years ago

Alright. I changed the title to better reflect what the goal of this thread is.

jamesplease commented 7 years ago

We'll likely need two builds for this – @tbranyen and I have been chatting about supporting this in libs at work 🤔

Godsenal commented 7 years ago

i had same problem yesterday. i tried

jamesplease commented 7 years ago

Thanks for the update, @Godsenal !


As an aside, I just updated this issue's title to be more accurate. You can use Waypoint as an ES6 module. This issue is specifically about react native support, which is an entirely different thing.

robhuzzey commented 7 years ago

I'm using react-waypoint in a basic app using webpack (not using React Native so not sure if I'm hijacking this thread) and still have this issue.

@jmeas @Godsenal I did the same fix suggested & it worked for me however I don't believe this is the long term solution?

trotzig commented 7 years ago

@robhuzzey I see, thanks for reporting. I wonder if things would just work ™️ if the .babelrc file wasn't there? Would you mind removing it manually (something like rm node_modules/react-waypoint/.babelrc) and trying to see if that makes things work?

We use babel to transpile the react-waypoint "bundle" (the file targeted in main from package.json). But if folks want to babel things themselves, we can just leave it up to them to configure. We can just exclude .babelrc when publishing the npm module.

jamesplease commented 7 years ago

@robhuzzey are you transpiling your node_modules with Babel using the Babel loader? If so, I would expect this error, but also, you shouldn’t need to do that. Most libraries, including Waypoint, are already transpiled, so you can blacklist the whole node_modules directory.

My guess is that, if you are currently transpiling your node_modules, then ignoring them will fix this issue, and make your build step much faster.

@trotzig , I agree that removing .babelrc from the node module makes sense.

bliang commented 6 years ago

Anyone had any luck with getting it to work with React Native or should I attempt to port it ? :)

Nantris commented 4 years ago

@jamesplease did the idea to have two versions ever go anywhere?