Skip to content
This repository has been archived by the owner on Mar 17, 2024. It is now read-only.

React Native support. #48

Closed
InterStelios opened this issue Oct 9, 2016 · 29 comments
Closed

React Native support. #48

InterStelios opened this issue Oct 9, 2016 · 29 comments

Comments

@InterStelios
Copy link

I am trying to use snoowrap in a React Native environment, but because its not running in a node environment I get this issue .

I would have thought that this was something that was fix as part of #33 ?

Is there a quick workaround for this?

@not-an-aardvark
Copy link
Owner

Hi, thanks for the issue. Unfortunately, I'm not sure I understand the problem (possibly because I'm not very familiar with React Native).

Could you clarify what you're doing, and what's going wrong?

@InterStelios
Copy link
Author

I think that snoowrap requires some node dependencies and because React Native is not running in a node environment, those dependencies do not exist.

@not-an-aardvark
Copy link
Owner

not-an-aardvark commented Oct 9, 2016

Could you be more specific about how you're installing/using snoowrap, and what errors are appearing? A setup where I can reproduce the issue would be ideal.

@InterStelios
Copy link
Author

So I am using a basic React Native project. I followed the official getting started instructions (mac+android).

Then I installed: npm install snoowrap -S and then import snoowrap from 'snoowrap'.

Then when the App restarts I get:

screenshot_20161009-100137

@not-an-aardvark
Copy link
Owner

Does this issue still happen if you install [email protected]?

Between 1.9.0 and 1.10.0, I made a change that causes the util module to not get included in a browser bundle to reduce the bundle size (since that module isn't needed for snoowrap to function correctly). However, it seems like browserify and react-native do different things here; browserify returns an empty object from require('util'), whereas react-native throws an error. snoowrap handles the empty object gracefully, but it doesn't handle a thrown error.

@InterStelios
Copy link
Author

I have tried [email protected] and there is an identical issue but for the url module. I have installed the url module but then the events module is missing. There might be a way to include most of the node shims (assuming that those are missing are node specific modules) but that might not be ideal.

So in the end, this is not an issue if there is and will not be support for react native.

@not-an-aardvark
Copy link
Owner

Hmm, I see. In theory, it would be nice if snoowrap could support react native, and I wouldn't mind doing it if it wouldn't pose a large maintenance burden.

At the moment, snoowrap uses the following modules from Node's standard library:

  • util to allow for better inspecting in Node's REPL (not necessary outside of Node, can continue gracefully if the module is unavailable)
  • url and querystring for parsing some URLs. (This could probably be replaced with a browser-specific parser if necessary. I considered using the built-in URL function, but at the moment it's not used because Safari 9 doesn't support URLSearchParams.)
  • fs to allow for easier image uploading with Node (possible to continue gracefully if the module is unavailable)
  • events for representing livethreads as EventEmitters.
  • http (indirectly, through request-promise) for sending requests. snoowrap already uses XMLHttpRequest instead if XMLHttpRequest is available, so I don't think this dependency should be causing any problems either way.

So you could probably solve the issues if you installed shims for each of these with something like rn-nodeify, but I acknowledge that this is not ideal.

@InterStelios
Copy link
Author

I will give rn-nodeify a try later today. Fingers crossed it will work.

@not-an-aardvark
Copy link
Owner

Friendly ping; were you able to resolve this issue?

@InterStelios
Copy link
Author

Hi I have tried using rd-nodeify and it does work, but then none of the commands work as expected. I am getting the following error:

screen shot 2016-10-18 at 20 57 40

I am pretty sure is something to do with the environment but i can't figure out.

@ucarion
Copy link

ucarion commented Dec 25, 2016

I'm not 100% sure if's the same problem, but I got the following error when using Webpack:

Module not found: Error: Cannot resolve module 'fs' in /home/ulysse/play/web/blueitt/node_modules/snoowrap/dist/objects

Which is caused when I try to import snoowrap from 'snoowrap'. Webpack tracks dependencies by looking at what is require-d in the source code, and can't find any module named fs.

Not sure what a good solution would be. For Webpack to realize fs isn't a necessary dependency, there cannot be require('fs') in the final source code, after any other processing (e.g. uglify). I don't see how you could achieve that.

The solution I'm using is to use the packaged snoowrap.js, although Webpack does complain about that:

WARNING in ./src/vendor/snoowrap.js
Critical dependencies:
3:1865-1872 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
 @ ./src/vendor/snoowrap.js 3:1865-1872

This workaround is totally fine for me. Since Webpack is a popular tool, perhaps this could be documented where you mention browserify in the README?

I'm still trying to figure out why, but I don't get the snoowrap object when I do import snoo from './snoowrap' -- window.snoowrap is there, but here is what console.log(snoo) produces:

screenshot from 2016-12-24 16-53-20

(Looks like it's the punycode module?)

Edit: Thanks for making snoowrap! It's an amazing piece of software. ❤️

@not-an-aardvark
Copy link
Owner

Thanks for the report. I think the issue is that snoowrap uses fs as an optional feature (to specify paths to files in functions like uploadStylesheetImage). Browserify silently ignores the missing fs dependency and returns an empty object, which works fine assuming the user doesn't try to use a filepath in the browser. On the other hand, it looks like Webpack throws an error when that happens.

I suppose I could trick Webpack by doing something like require('f' + 's'), but I'm not sure whether that would cause runtime errors. I'm not very familiar with Webpack, so if there's an idiomatic solution to this problem then I'd be happy to add it.

In response to your other question, I'm not sure why import snoo from snoowrap would be returning the punycode module. (snoowrap doesn't use punycode, although it's possible it's getting inserted by browserify somewhere.) My best guess is that browserify does something odd since the prebuilt version of snoowrap is intended to be used as a top-level module.

@ucarion
Copy link

ucarion commented Dec 29, 2016

I don't think the 'f' + 's' trick would work. Webpack parses require in a non-trivial way.

Do you ever do module.exports = snoowrap? That might make Browserify do the right thing.

@not-an-aardvark
Copy link
Owner

Do you ever do module.exports = snoowrap? That might make Browserify do the right thing.

Yes, I do that here.

I've been looking at this thread. Apparently it might be possible to resolve this by excluding fs from your webpack config.

What happens if you modify snoowrap's package.json and add "fs": false to this list. (If it works, I can update it in the repo as well.) That seems like it would be the easiest solution, since it would fix the problem for everyone.

@ucarion
Copy link

ucarion commented Dec 29, 2016

I could definitely have Webpack ignore fs. The idea was to see if snoowrap would automatically work with Webpack, without having users modify their config. Plus, I think ignoring fs could have some unwanted downsides: if later on I add a new package to my project that really needs fs, Webpack will silently ignore the dependency instead of giving me an error.

I don't know how to use a local copy of snoowrap in my project.

@not-an-aardvark
Copy link
Owner

To clarify, I meant to ask what happens if you monkeypatch node_modules/snoowrap/package.json to add that line. (The change would be overwritten the next time you run npm install, but it would be useful to figure out whether that line makes a difference.)

@ucarion
Copy link

ucarion commented Dec 30, 2016

Yes, that does the trick! I'll try re-writing my code without the window.snoowrap to see if everything works as expected.

@not-an-aardvark
Copy link
Owner

Great! Let me know if you run into any other Webpack-related issues, otherwise I'll publish a new version of the package with that line added sometime in the next few days.

@maciebey
Copy link

After some trial and error I actually was able to get Snoowrap working with react native using rn-nodeify. Initialized project by the steps below and everything seems to be working as intended.

react-native init myProject 
npm install --save snoowrap 
npm install --save asyncstorage-down  
npm install --save-dev mvayngrib/rn-nodeify
./node_modules/.bin/rn-nodeify --hack --install

https://github.com/maciebey/rnSnoowrapExample

@hhtran
Copy link

hhtran commented Jul 30, 2017

Hello,

Has there been any progress towards react native support lately?

Even with the suggestion from @maciebey I still get the error from Timing.createTimer from this image: https://cloud.githubusercontent.com/assets/4718153/19494137/91ada186-9575-11e6-9c4f-cb73d50e5207.png

This occurs only on android and not ios

@not-an-aardvark
Copy link
Owner

I don't use React Native, so working on this issue hasn't been a priority for me. However, if you figure out what needs to be changed in the package and/or make a PR with the changes, I'd be happy to accept it. Right now I'm not familiar with how react native works or its available APIs, and I don't have a development environment set up for react native, so it's difficult for me to debug this issue.

@arrygoo
Copy link

arrygoo commented Nov 11, 2017

Same issue only on Android.
Would be so cool if this package started having react-native support.

@markspolakovs
Copy link
Contributor

markspolakovs commented Aug 8, 2018

The way I (somewhat naively) see it, each of the native Node modules can either be replaced with a npm package or bypassed:

I'll try and set up a RN environment and have a crack at these, but I make no promises. If I get hit by a bus though, this should be a reasonable starting point.

EDIT: Subreddit.upload* uses stream, which we'll have to bypass as well.

@danhab99
Copy link

Hi, I was wondering if there was any progress on this issue? If not, dose anybody know about alternatives to snoowrap?

@markspolakovs
Copy link
Contributor

@danhab99 I can't speak for the maintainers or other devs but personally I haven't had an opportunity to work on it.

Reddit's API is mostly just standard REST and OAuth, so you could in theory use any library that supports those (or, heck, just fetch() directly) - snoowrap is just a nicer interface for them.

@danhab99
Copy link

danhab99 commented Sep 2, 2018

I think I have a solution. I talked with some people on discord and we figured it out, @stelioskiayias screenshot where React was complaining about Snoowrap trying to use the util package said it all, we just weren't listening.

Whenever Snoowrap would try to use a node package that wasn't available, I just installed it.

Try running:

npm i -S fs events path stream url util

@danhab99
Copy link

Issue #243 seems to imply that my solution worked. Unless we want to continue to discuss this issue, I recommend closing.

@Venefilyn
Copy link
Collaborator

Venefilyn commented Nov 20, 2019

Sounds good to me. Feel free to reply again and we can re-open the issue.

Solution:
Install:
npm i -S fs events path stream url util base-64

Add base 64 globals in your entry file:

import {decode, encode} from 'base-64'

if (!global.btoa) {
    global.btoa = encode;
}

if (!global.atob) {
    global.atob = decode;
}

After snoowrap initialization change _nextRequestTimestamp

const snoowrap = new snoowrap({/**...*/});
snoowrap._nextRequestTimestamp = -1;

@alexjball
Copy link

I ran into a minor issue while using fromApplicationOnlyAuth: xhr makes a call with a large timeout (2^32), which triggers a warning on Android. Subclassing snoowrap to pass in the timeout fixes things. Call with NativeSnoowrap.fromApplicationOnlyAuth:

class NativeSnoowrap extends snoowrap {
  constructor(options) {
    super(options);
    this._nextRequestTimestamp = -1;
  }

  credentialedClientRequest(options = {}) {
    return super.credentialedClientRequest({timeout: 60e3, ...options})
  }
}

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

No branches or pull requests

10 participants