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] Configure the locations from which modules can be imported #3099

Closed
mschwartz opened this Issue Sep 28, 2015 · 43 comments

Comments

Projects
None yet
@mschwartz
Copy link

mschwartz commented Sep 28, 2015

require() and import are really important features that deserve some love, IMO.

Forcing us to use relative paths in all our import/require statements is not ideal. It works only ok for the react demos that have a handful of files all in the root. For example, the Movie example in this repo:

image

The require on line 19 works because you've basically hardcoded require to search node_modules/ where react-native exists.

The require on line 26 forces you to put a ./ on the beginning of the path, and it works, but not ideally as I said.

Consider you want to actually break up this file: https://github.com/facebook/react-native/blob/master/Examples/Movies/SearchScreen.js

One file per component, even if they're private (for now). Reusability is the goal, no?

If you move this file into a directory called "components/", now you have to go and edit the require() statement to reflect the new path.

Things get really ugly if you want to organize your project into a more complex structure. Consider search/component1, search/component2, details/movie1, details/movie2, and so on. Now you're relative paths, if they are sharing code, are ../search/whatever, ../details/whatever. Moving these files around to reorganize your project now means you have to deduce what the new relative paths must be and you have to edit all those files.

I suggest you think about implementing a robust require() implementation. I suggest at least the following features:

require.paths array

This is an array of search paths used by require. It searches the paths in the order of the array. It is allowed by the CommonJS require specification.

Example:

// In your index.ios.js at the very top:
require.paths.unshift('search');
require.paths.unshift('movies');

Now you can simply require('component1) ANYWHERE in your source code and search/component1 will be used. You can move your files around without having to edit your require() statements in every one. Just change the require.paths.

Support directories

If you require('foo') and foo is a directory, you should:

  1. Look for index.js and use that to locate the module requested.
  2. THEN Look for package.json and use that to locate the module requested.
  3. THEN look for bower.json and use that to locate the module requested.
  4. THEN look for the module requested by filename (e.g. require('foo').bar means foo/bar.js)

Benefits

If you implement these changes, people will be easily able to share components via mechanisms other than npm. You will easily be able to copy components from one project to another without having to refactor the require() statements in each.

Please consider this.

@ide

This comment has been minimized.

Copy link
Collaborator

ide commented Sep 28, 2015

Do systems like Webpack and Browserify support require.paths? I thought Node deprecated that feature awhile ago too and asks for NODE_PATH. It's been a few years since I've looked into this so perhaps things have changed.

Generally the packager needs to be able to resolve requires statically so it can produce a bundle. So we could support NODE_PATH or a similar kind of configuration at compile-time but I don't know how we'd support require.paths because client code doesn't have the same capability as server code in this way.

@mschwartz

This comment has been minimized.

Copy link

mschwartz commented Sep 28, 2015

NODE_PATH would be fine.

However, I don't think there's an issue client vs. server.

On the client, you can still use search paths.

2015-09-28 at 11 14 am

Instead of modulesMap[id], you'd loop through require.paths and try modulesMap[require.paths[i] + id) until found. Or not found...

@mschwartz

This comment has been minimized.

@nevir

This comment has been minimized.

Copy link
Contributor

nevir commented Sep 28, 2015

Couple caveats to consider:

  1. NODE_PATH is processed only when Node starts. You cannot modify it at runtime. (i.e. everyone has to be very careful to set up their environment properly for all entry points. packager, tests, etc)
  2. A lot of existing tooling would not be not aware of this (linters, webpack, etc) and will need to add support for it.

Also, there's a great thread for and against this pattern over at https://gist.github.com/branneman/8048520, which might lend some more insight

@ide

This comment has been minimized.

Copy link
Collaborator

ide commented Sep 28, 2015

The key thing is that (some of) the configuration needs to happen at compile-time because we need to create a bundle. NODE_PATH and Webpack's resolve config meet this requirement... I prefer the latter because the config is more visible than NODE_PATH but they're isomorphic from what I can tell.

Another important point to consider is how this will interact with ES6 import and ES7 System. I believe they are compatible with a NODE_PATH-like option but something to keep in mind.

@mschwartz

This comment has been minimized.

Copy link

mschwartz commented Sep 28, 2015

NODE_PATH would be a huge win.

I was just looking at webpack myself and saw the resolvers configuration options. Another win.

@ide ide added the packager label Sep 28, 2015

@mschwartz

This comment has been minimized.

@nevir

This comment has been minimized.

Copy link
Contributor

nevir commented Sep 28, 2015

Take a look at the ES7+ spec loader, too (System.js will layer on top of it): https://github.com/whatwg/loader

@mschwartz

This comment has been minimized.

Copy link

mschwartz commented Sep 28, 2015

Excellent. See 10.3.1.

node_modules is not in their tbd search path ...

FWIW

On Monday, September 28, 2015, Ian MacLeod notifications@github.com wrote:

Take a look at the ES7+ spec loader, too (System.js will layer on top of
it): https://github.com/whatwg/loader


Reply to this email directly or view it on GitHub
#3099 (comment)
.

@brentvatne brentvatne changed the title Implement require() properly [Packager] Implement require() properly Sep 30, 2015

@mschwartz

This comment has been minimized.

Copy link

mschwartz commented Oct 1, 2015

http://webpack.github.io/docs/configuration.html#resolve-alias

Read on in that link...

Read the through the comment thread on that gist linked by @nevir ...

Node's require is poorly implemented. No reason to have a bad require for react-native.

IMO

@ide ide changed the title [Packager] Implement require() properly [Packager] Configure the locations from which modules can be imported Oct 1, 2015

@ide

This comment has been minimized.

Copy link
Collaborator

ide commented Oct 1, 2015

Might be possible to implement an offline version of this: https://github.com/systemjs/systemjs/blob/master/docs/config-api.md#map

@mschwartz

This comment has been minimized.

Copy link

mschwartz commented Oct 1, 2015

I like it.

@JaapRood

This comment has been minimized.

Copy link

JaapRood commented Nov 11, 2015

Just a question: would support for symlinks help you out with this problem? I haven't read through all the material linked about different module systems, all I know is that with React Native being based on CommonJS (node) and relying on NPM it'll be quite the change to break from that. While that may or may not be inevitable in the future is not a question I'm looking to answer or claim I have answer to, but perhaps the symlink support is a more short term achievable goal.

@marcshilling

This comment has been minimized.

Copy link

marcshilling commented Nov 11, 2015

You can use an absolute path on imports/requires:

import {ProximaNovaText} from 'MyApp/src/components';
require('MyApp/src/utils/moment-twitter');

where 'MyApp' is whatever name you registered in your index.ios.js file

@grabbou

This comment has been minimized.

Copy link
Collaborator

grabbou commented Dec 10, 2015

Well, looks like this has improved slightly in latest versions. As long as your folder added to rootFolders have package.json inside it, you can use the name from that package.json instead of a relative path.

See my structure with React Native 0.15 below:

assets/
core/package.json (@g/core name in package.json)
platform/mobile/package.json (@g/mobile name in package.json)
platform/mobile/node_modules/react-native
platform/web/package.json (@g/web - ignored as not included in mobile app)

packager starts from platform/mobile with ../../core added to roots with a help of rn-cli.config.js that also works for bundle command:

const defaultConfig = require('react-native/local-cli/default.config.js');

module.exports {

  getProjectRoots() {
    const roots = defaultConfig.getProjectRoots();
    roots.unshift(path.resolve(__dirname, '../../assets'));
    roots.unshift(path.resolve(__dirname, '../../core'));
    return roots;
  }

};

I can see the paths getting transformed from:

require('../../core/redux/reducer.js')

to

require('@g/core/redux/reducer.js')

and

require('./index.ios.js')

to

require('@g/mobile/index.ios.js')

which basically means package name from package.json is used which works exactly as you would've npm link them all together. So who needs symlinks then?

@brentvatne @mkonicek is that how packager works or have I just entered an edge-case that I should not rely on?

Something is definitelly in the air since when I set up the same name for core and mobile (e.g. acme) I get:

{"type":"InternalError","message":"react-packager has encountered an internal error, please check your terminal error output for more details"}

Note Looks like the approach for assets is slightly different:

require('../../../../../assets/a.png')

is converted to

require('/Users/grabbou/project/assets/a.png')

However, when you have package.json in your assets as well (@g/assets name in package.json), you could write

require('@g/assets/a.png')

which is equivalent.

@jsierles

This comment has been minimized.

Copy link
Contributor

jsierles commented Mar 20, 2016

Hey @mschwartz! Thanks for bringing this issue up.
I'm closing this sine we're trying to focus Github issues on bugs, and this is potentially solved by @grabbou's solution. Feel free to submit a PR or add a Product Pains entry to get more attention on this issue.

@rulatir

This comment has been minimized.

Copy link

rulatir commented Apr 28, 2016

The ability to define import paths is an absolutely essential part of any programming environment that "has" even the vaguest notion of "import". Its absence from React Native is a bug-level deficiency. I request reopening this issue. Product Pains looks like a toy; I am unconvinced that it receives any actual attention.

@grabbou

This comment has been minimized.

Copy link
Collaborator

grabbou commented Apr 28, 2016

Have you seen my article on Medium how to define import paths? https://medium.com/@grabbou/a-cure-for-relative-requires-in-react-native-2b263cecf0f6 It works well :)

Also Product Pain was working really well recently and we closed one of the most up voted issues - see this for more details https://www.facebook.com/groups/react.native.community/permalink/759574700844777/

@rulatir

This comment has been minimized.

Copy link

rulatir commented Apr 28, 2016

It doesn't work well for inherited codebase with dozens of files, most of them importing several other files from the same codebase. Your solution would still require rewriting every last one of those 500+ import lines, it would just require rewriting them with something that only depends on the imported module's location and not on the importing module's location.

@miqmago

This comment has been minimized.

Copy link

miqmago commented Apr 29, 2016

@grabbou the problem with this solution is that it breaks intellisense, for example, in visual studio (don't know the rest of gui). Then it cannot find where is the file, which are the properties and worse of it, you cannot jump to class definition with cmd+click

@grabbou

This comment has been minimized.

Copy link
Collaborator

grabbou commented Apr 29, 2016

Ah ok, didn't know. I am mainly using flowtype with Nuclide and that works well there (just in case someone's interested)

@grabbou

This comment has been minimized.

Copy link
Collaborator

grabbou commented May 11, 2016

Please vote on Product Pains instead, that way we can prioritise the issues.

@dutzi

This comment has been minimized.

Copy link

dutzi commented May 26, 2016

A simple solution I found was creating a very minimalistic package.json inside the top-most folder you want to absolutely import from. That package.json should look like this: { "name": "src" }, where "src" is the name of that folder.

The you can simply do import X from 'src/X.js' just as you would normally do.

@miqmago

This comment has been minimized.

Copy link

miqmago commented May 27, 2016

Thanks @dutzi it's not perfect but it works!

@rclai

This comment has been minimized.

Copy link
Contributor

rclai commented Aug 18, 2016

@marcshilling, is that MyApp prefix coming from whatever name you registered with AppRegistry or is it the name in package.json that @dutzi is talking about?

@aleclarson

This comment has been minimized.

Copy link
Contributor

aleclarson commented Sep 12, 2016

Have you tried pasting @providesModule SearchScreen at the top of SearchScreen.js?
Not sure if that works, but (if it does) you would be able to do require('SearchScreen'), thus avoiding any import path refactoring.

@mmazzarolo

This comment has been minimized.

Copy link

mmazzarolo commented Sep 12, 2016

Have you tried pasting @providesModule SearchScreen at the top of SearchScreen.js?
Not sure if that works, but (if it does) you would be able to do require('SearchScreen'), thus avoiding any import path refactoring.

It won't work on tests though :(

@aleclarson

This comment has been minimized.

Copy link
Contributor

aleclarson commented Sep 12, 2016

@mmazzarolo Isn't that a Jest issue, though?

@aleclarson

This comment has been minimized.

Copy link
Contributor

aleclarson commented Sep 27, 2016

This issue seems relevant.

@evgenyrodionov

This comment has been minimized.

Copy link

evgenyrodionov commented Jan 29, 2017

@grabbou is this solution working for react-native 0.40?

@vinhlh

This comment has been minimized.

Copy link

vinhlh commented Mar 3, 2017

I miss the way webpack alias does so much :"(

import Navigation from '@components/Navigation'
import navigationReducer from '@reducers/navigation'
@stochris

This comment has been minimized.

Copy link

stochris commented Mar 21, 2017

Doesn't this work just as well ?
I've been using it on a complex project, and it's working pretty good. The only caveat is that you need to update all of the other IDE settings ( vs setting the project name in package.json, which I would think works out of the box )

@ide

This comment has been minimized.

Copy link
Collaborator

ide commented Mar 21, 2017

Yeah. Babel plugins are generally easier to grok and author. One caveat is that if you change them you need to clear the packager cache but they work well otherwise.

@yugalbagul

This comment has been minimized.

Copy link

yugalbagul commented May 12, 2017

Unfortunately @dutzi's solution does not seem to work with react-native@0.44.0.

Also this post here says it is not allowed to fetch from outside of your React Native Project root.

I have followed this medium article and other such sources which suggest similar way.

I am running -
"react": "16.0.0-alpha.6", "react-native": "0.44.0",

Folder Structure

  -- parent
    -- package.json {"name":'parent'}
    -- mobile
      -- app
        -- modules
          -- login
            -- Login (import { loginAPi } from 'parent/src/data/api')
    -- src
      -- package.json {"name": "source"}
    -- data
      -- package.json {"name": "data"}
      -- api
        -- package.json ({"name":"api"})
        -- index.js

packager console:

error: bundling: UnableToResolveError: Unable to resolve module `parent/src/js/data/api` from `/Users/../../../../mobile/app/modules/login/Login.js`: 
Module does not exist in the module map or in these directories:
/Users/../../../parent/mobile/node_modules/parent/src/js/data
/Users/../../../parent/node_modules/parent/src/js/data
/Users/../../../node_modules/parent/src/js/data

NOTE- Web application which is in src has been already developed and has considerable code, and the team is just starting with the react-native set up. We want to use the data part of the web application in the mobile.

Need help getting things right.

@maotora

This comment has been minimized.

Copy link

maotora commented Jun 7, 2017

@yugalbagul did you solve this??

I'm currently getting the similar error and would like to have some help.

Thanks.

@kylebebak

This comment has been minimized.

Copy link

kylebebak commented Jul 12, 2017

@yugalbagul @maotora
I can also confirm @dutzi 's solution doesn't work if the minimal package.json file is in a parent directory relative to the RN project root. From what I understand, this is expected behavior of the RN packager.

I haven't found a work-around.

@jamesone

This comment has been minimized.

Copy link

jamesone commented Jul 31, 2017

As of 0.46 I have noticed that trying to import by the name set in package.json does not work. It throws: Unable to resolve module 'img/my_image_name' from 'path/path_of_import': Module does not exist in module map.

@RocketeerFly

This comment has been minimized.

Copy link

RocketeerFly commented Jan 9, 2018

Thanks @dutzi
but seems VSCode doesn't understand this trick. Hence, suggestion & Ctrl+Click is not working for the imports by this way

@lanlin

This comment has been minimized.

Copy link

lanlin commented Mar 12, 2018

Hi @ALL, here's my way to solve this. This may be a little help.

CRNA (create-react-native-app) module resolve & eslint rules check

@manuelbertelli

This comment has been minimized.

Copy link

manuelbertelli commented Mar 25, 2018

I need to give a simple explanation because I've tried many ways to accomplish this simple goal of having a separate folder for my files and make packager understand where everything was located. As @dutzi said but wasn't totally clear that YOU NEED TO HAVE package.json even inside folders that were only folders of folders. In this example, we have the following structure:

.env
-- src <folder, doesn't need package.json as it is declared in .env>

  -- components <folder>
   - package.json

    -- button <folder>
     - package.json
     - button.js
  1. Add a .env file to root folder and specify NODE_PATH=src (or your files folder)
  2. Inside EVERY FOLDER (even if it is a folder of folders, and here is the trick!) under src (as in this example) you need to have a package.json file with at least two pieces of information. In this example, components folder should have:
{
    "name": "components",
    "private": true
}
  1. Folders that have JS files, i.e. that aren't folders of folders, you need to specify main attribute in package.json, e.g button component:
{
    "name": "button",
    "main": "./button.js",
    "private": true
}
@valefor

This comment has been minimized.

Copy link

valefor commented Apr 25, 2018

try this:
https://github.com/tleunen/babel-plugin-module-resolver

don't forget to reload packager with '--reset-cache'

@stephenhuh

This comment has been minimized.

Copy link

stephenhuh commented Jun 22, 2018

Relevant link:
Frozen require resolution algorithm

The website seems to suggest (as I read it) the package.json approach first, then adjusting NODE_PATH second.

NODE_PATH was originally created to support loading modules from varying paths before the current module resolution algorithm was frozen.

@facebook facebook locked as resolved and limited conversation to collaborators Jul 21, 2018

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