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

[Packager] should use compatible versions of node core modules #1871

Closed
apaleslimghost opened this issue Jul 4, 2015 · 11 comments
Closed

[Packager] should use compatible versions of node core modules #1871

apaleslimghost opened this issue Jul 4, 2015 · 11 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@apaleslimghost
Copy link

Many npm modules are currently not compatible with React Native because they depend on node's core modules. A good example of such a module is through, which depends on the stream module, and has nearly 1500 modules that depend on it. Currently, if a module like through is depended on, you get a slightly opaque runtime error such as Requiring unknown module 'stream', with no further information.

Browserify uses various npm modules in place of node core modules. I guess Webpack must be doing something similar.

Ideally, I'd like to see:

  • Packager resolving node modules with a list akin to Browserify's
  • "unknown module" errors be promoted to compile-time, with helpful debugging information, possibly including the module hierarchy that led to the unknown module

What do you think?

@brentvatne brentvatne changed the title Packager should use compatible versions of node core modules [Packager] should use compatible versions of node core modules Jul 7, 2015
@brentvatne
Copy link
Collaborator

Seems reasonable... this might have to be community driven, if you were to submit a PR it would be very helpful to get this moving along further

@ide
Copy link
Contributor

ide commented Jul 7, 2015

Could you define a set of polyfills and require them from your index.js file? Seems simpler and more amenable to evolution to me.

@apaleslimghost
Copy link
Author

@ide The problem is when some module deep in your dependencies does e.g. require('stream'). Packager doesn't know about that module so the entire bundle fails. The app never even gets to index.js, so there's no way to polyfill there (even if I could hook into require somehow).

@apaleslimghost
Copy link
Author

@brentvatne I'm not familiar with Packager's internals, any pointers for where this should go?

@brentvatne
Copy link
Collaborator

@quarterto - unfortunately I'm very much in the same boat as you

@mvayngrib
Copy link
Contributor

+1, would appreciate a pointer for where in packager it would be best to do this.

I would love for react-native to accept a "fallbacks" mapping, i.e. if some module does require('fs'), and 'fs' can't be resolved, check fallbacks.

Browserify and webpack currently (unfortunately) make the choice for you with core module fallbacks, so if you want to add some that are missing (fs, dgram, net, a few others), or choose different fallbacks for a different environment -- browser vs chromeapp vs react-native, etc. -- there's no convenient way. I'm currently using react-native-webpack-server and a fork of node-libs-browser that adds fs, dgram and others. It works, but it's not terribly convenient. I'd prefer to be able to use react-native's packager.

Edit: correction, browserify does allow you to pass in alternative builtins

@mvayngrib
Copy link
Contributor

@amasad maybe you're the one to give us a hint? :)

@amasad
Copy link
Contributor

amasad commented Sep 8, 2015

hey @mvayngrib. Currently, we only support "script polyfills". Meaning they run before the require library. We can potentially have a way to define "core" modules. Support for this needs changes across the resolver and the bundler parts of the packager. Feel free to dig into the packager code (found in packager/react-packager) and try to figure it out. Let me know if you have any specific questions.

Also, FYI, we're moving the packager to it's own repo some time soon, so I'll ping you to move the discussion there.

@zsszatmari
Copy link

+1

@jsierles
Copy link
Contributor

A more detailed discussion here #6253. To keep issues more focused, I'm closing this in favor of discussion there. @amasad @mvayngrib feel free to reopen here or discuss there.

@aleclarson
Copy link
Contributor

git clone https://github.com/facebook/react-native.git
cd react-native
npm install --save path-browserify

mkdir Libraries/NodeJS
touch Libraries/NodeJS/path.js

Set the contents of Libraries/NodeJS/path.js to:

/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 *
 * @providesModule path
 */

module.exports = require('../../node_modules/path-browserify');

This technique can be applied generically.
But I suppose this should be possible without cloning react-native.

Any thoughts?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

9 participants