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

Support absolute paths with packages/ folder #741

Closed
gaearon opened this issue Sep 24, 2016 · 86 comments
Closed

Support absolute paths with packages/ folder #741

gaearon opened this issue Sep 24, 2016 · 86 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2016

We’ve been using Lerna in Create React App, and I really like its approach. I think we should support a similar workflow (even without Lerna itself) for the “absolute paths” feature.

I imagine it working like this:

  • In addition to src, you can create a special top-level folder called packages.
  • Inside packages, you can create folders like app, stuff, lol, and they all “see” each other so you can import things from app/whatever.js or lol/wow.js. You can also import any packages from src (but not vice versa). The entry point is still src/index.js.
  • We won’t add any magic handling that breaks Node resolution mechanism. Instead, whenever you npm start, npm test, or npm run build, we will run a utility that creates symlinks from node_modules of the root project to every folder in packages. It reports a hard error if there is a conflict. This means the authors can add server rendering after ejecting without scratching their heads, and that all the tooling assuming Node resolution mechanism keeps working.
package.json
public/
src/
  index.js # can import app/banana.js or harry-potter/wand.js
packages/
  app/
    banana.js # can import harry-potter/wand.js
  harry-potter/
    wand.js # can import app/banana.js
node_modules/
  app -> packages/app
  harry-potter -> packages/harry-potter
  other-deps

Am I missing why this would be a bad idea?

@gaearon gaearon added this to the 1.0.0 milestone Sep 24, 2016
@udfalkso
Copy link

We've been using linklocal for this type of thing.

https://github.com/timoxley/linklocal

@kasperpeulen
Copy link
Contributor

What problem would this solve? I see lerna as being useful as for example now users can easily use the eslint config of create-react-app in any package they want.

Could I publish this package harry-potter to npm? So that I could use it another package as well? Is that the idea?

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

How does this help? Can anyone who will benefit from this comment?

@amandapouget
Copy link

This seems like an acceptable solution to me, if well documented. It sounds like under this proposal that it would make sense to put most domain models under packages/ and so in an OO system, one would not have much under src/, though tests/ should still go under src/ and import domain models under test from packages.

That's a clear change from what we're used to thinking, that 'everything goes under source'. Am I holding it right?

Also, it sounds like things under src/ would not be accessible via absolute paths, only things under packages/. This should be made really clear in the documentation... We have a folder called src/testHelpers. Implementing this solution would mean moving it to packages (in order for src/banana.tests.js to be able to import a testHelper file by absolute path).

So I guess in the final analysis, if anyone has been using the NODE_PATH=src workaround, to implement this solution, one would have to reorganize all the folders and rewrite the imports. Is there a solution that would keep the same folder structure people have already built with create-react-app?

@amandapouget
Copy link

amandapouget commented Sep 24, 2016

The problem we're trying to solve is:
import 'path/to/banana.js' instead of the brittle import '../../banana.js'.
Absolute paths to your app's files instead of relative paths.

The current workaround is to add an environment variable before running commands:
NODE_PATH=src/ && npm run start

But apparently some people have problems with that workaround, perhaps because of a conflict between the name of their file and the names of folders and files under node_modules/. Though honestly I can't help but wonder if that's an edge case.

@amandapouget
Copy link

@gaearon Why not just point to src/ instead of adding packages? I'm sure there is a great reason that's just not clear to me yet.

@kasperpeulen
Copy link
Contributor

Why not just point to src/ instead of adding packages? I'm sure there is a great reason that's just not clear to me yet.

I was wondering this to, you could also symlink any directory in src. And then still keep src/index.js as entry point. No?

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

Why not just point to src/ instead of adding packages? I'm sure there is a great reason that's just not clear to me yet.

@mandysimon88 that is what we are talking about. Every file in your project can refer to some other file. You need absolute paths in every file so this solution (in some sense) forces you to put everything in packages/.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 24, 2016

How does this help? Can anyone who will benefit from this comment?

Please refer to discussion in #636.

Why not just point to src/ instead of adding packages? I'm sure there is a great reason that's just not clear to me yet.

It is too brittle in my experience. People won’t realize what is happening unless there is some explicit opt-in, and I think packages is a good way to signal the intention, as well as to make people think twice about how to name root folders (instead of accidentally aliasing something that already exists in node_modules).

I also don’t want beginners to do this, which is why I want this feature to be opt-in. IMO it’s useful in larger projects, not from the first day.

Is there a solution that would keep the same folder structure people have already built with create-react-app?

You wouldn’t have to change anything. This feature would be strictly additive. You could migrate to it one helper at a time, if you’d like, or even keep using NODE_PATH. Or you could copy all top-level folders from src into packages and have src/index.js re-export whatever package you like, thus providing a quick migration path.

@kasperpeulen
Copy link
Contributor

kasperpeulen commented Sep 24, 2016

It is too brittle in my experience. People won’t realize what is happening unless there is some explicit opt-in, and I think packages is a good way to signal the intention, as well as to make people think twice about how to name root folders (instead of accidentally aliasing something that already exists in node_modules).

Just some reference how an other language solved this problem. Because what Dart does is something very similar as you are proposing @gaearon. It will see anything in the lib folder as a regular installed package where the name of the package is gotten from how you call the app in package.json. Say the app is called my-app then:

package.json # name: my-app
public/
src/
  index.js # can import my-app/fruits/banana.js, my-app/harry-potter/wand.js or my-app/dom.js
lib/
  dom.js # top level file, can be imported as `my-app/dom.js`
  fruits/
    banana.js # can import my-app/harry-potter/wand.js
  harry-potter/
    wand.js # can import my-app/fruits/banana.js
node_modules/
  my-app/
    dom.js -> lib/dom.js
    fruits -> lib/fruits
    harry-potter -> lib/harry-potter
  other-deps

The adventage of this, is that you don't have to worry about how you would call the directory in the lib (or packagedirectory).

@jfmengels
Copy link

Should you go ahead and do this, you should configure eslint-plugin-import to resolve files correctly. I see that the rules are currently disabled, but that's something that should be done at some point.

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

How does this help? Can anyone who will benefit from this comment?

Please refer to discussion in #636.

That issue is where I came from. This solves the problem stated in the original request terribly at best.

Take the f8app, stop using @providesModule and apply this solution. Can you imagine ending up with a better structure?

@gaearon
Copy link
Contributor Author

gaearon commented Sep 24, 2016

terribly at best.

This is a pretty strong statement. Can you help me understand what is “terrible” about this proposal?

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

To clarify, it is terrible for being a solution to the original request in #636.

This solution basically suggests that we should move stuff we want to import using absolute paths to a directory in packages/. The problem is that we want to import almost everything using absolute paths. If I were to use this pattern, I would have to move my actions, components, helpers, 18n, theme.js, env.js and config.js to somewhere in packages. That's my whole app!

@Pajn
Copy link
Contributor

Pajn commented Sep 24, 2016

That's my whole app!

That's the whole point! Structure your app as packages instead of a monolith

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

@Pajn it already is. If I had to use this pattern, I'd just move some directories to packages and it would work, but then src would be almost empty. What's next? We remove src?

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

Or are you expecting me to make every component a package? Am I supposed to have a thousand React components laid flat in packages?

@modernserf
Copy link

Could this be done without copying files by putting packages in src/node_modules (see #607) and checking for conflicts?

@AlicanC
Copy link

AlicanC commented Sep 24, 2016

@modernserf yes, but don't you think that's a bit ugly?

  • A developer would and should expect a directory called node_modules to be managed by npm.
  • CRA targets beginners. What happens when a beginner faces with a problem and reads on SO that they should try removing node_modules and doing an npm i? The original node_modules might even be hidden by the editor. "Why do they ask me to remove my code?"
  • Most editors, libraries and .gitignores do not follow the best practice and treat all directories named node_modules just like the one at the root. Even React Native has this wrong. (It should've been /node_modules/ and not node_modules/.)

You should just treat node_modules as a reserved name.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 25, 2016

@AlicanC

Wouldn't having a single folder called "app" in packages satisfy your use case? You could even have src/index.js reexport app/index.js and then any module is addressable as app/whatever.

Of course it's a bit more typing than if you give implicit namespaces but I think explicitness is favourable in this particular case.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 25, 2016

it already is. If I had to use this pattern, I'd just move some directories to packages and it would work, but then src would be almost empty. What's next? We remove src?

Then it would be mostly empty for you. I don't see a problem here. You choose a convention where everything lives in an absolutely addressable package. That's fine but then people new to your codebase need to know where the entry point is. So it makes sense to me that src stays at top level because that'll be the first place people look (and they can see what you reexport if you really decide to move everything into packages).

@AlicanC
Copy link

AlicanC commented Sep 25, 2016

I don't like it, but sure.

Which official Facebook projects can we expect to see dogfooding this pattern?

@cecilemuller
Copy link

cecilemuller commented Sep 25, 2016

A developer would and should expect a directory called node_modules to be managed by npm.

It's a common misunderstanding: node_modules does not imply "managed by NPM", it's just how Node natively resolves paths, hence why NPM uses the name.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 25, 2016

Which official Facebook projects can we expect to see dogfooding this pattern?

As you probably know, Facebook is using Haste everywhere in its product code. You might have noticed it is not very popular in the open source community. Some things that work well for Facebook also require a lot of infrastructure that others don’t have.

Create React App explicitly does not follow everything the way Facebook does it. In fact, it would not exist if it followed the usual dogfooding principle because, for example, Facebook doesn’t actively use Webpack and instead has super powerful development servers in the cloud that compile the code. The situation in the open source community is just different, and with this project we tried to break our usual principles and go where people are. I think the success of this project speaks that breaking the rules is a good idea sometimes.

That said, project structure with packages is used by Babel, Jest, and Create React App itself. I don’t see why it wouldn’t work for apps, especially as it is pretty much the same thing as absolute imports (requested numerous times and enabled in many popular boilerplate projects), but more explicit.

@hswolff
Copy link

hswolff commented Sep 27, 2016

I'm a big fan of this approach. It encourages encapsulation of like logic for a local package used in the packages folder.

For example I have a packages/components folder that holds all my dumb/presentation React components. This lets me safely know that any component found in there must be given properties to configure its behavior and likewise has no reliance on whatever state management you're using.

Further it also creates a structure such that if you wanted to open source components inside of packages the migration path is a lot more straightforward as you're already considering them as a package.

Big fan of this approach.

@sirbrillig
Copy link

Coming from the rather large React codebase of wp-calypso where we heavily rely on absolute imports, I think this is a great idea. Although not necessary at all for a small project, when a project becomes large, absolute imports have a huge value and can save a lot of developer time.

Having this be an option in CRA may help provide a way for new developers to encounter the concept in a friendly way with a suggested implementation rather than trying to discern how to do it themselves (or giving up since there is no "right way").

@gaearon
Copy link
Contributor Author

gaearon commented Jul 9, 2017

We’re going to keep supporting NODE_PATH=src as described in earlier comments.
This is enough to keep this issue at bay for now.
We might revisit this later.

@mizx
Copy link

mizx commented Aug 4, 2017

For those using react-scripts-ts:

  1. add NODE_PATH=src/ to the .env file (thanks will-stone!)
  2. add "baseUrl": "./src" to your tsconfig.json file.

@jasan-s
Copy link

jasan-s commented Aug 14, 2017

Using the above accepted solution I still can't achieve the following:
import { componentA, componentB } from 'components'
instead I still have to do the following:
import componentA from 'components/componentA/componentA'
import componentB from 'components/componentB/componentB'

My component structure is the following:

 src
  components
     componentA  
       componentA.js
     componentB  
       componentB.js
     index.js

@will-stone
Copy link

@jasan-s as support questions probably belong on StackOverflow, can you paste your question there and include the contents of your index.js file?

@leob
Copy link

leob commented Aug 14, 2017

@jasan-s I think that should work if the contents of your 'components/index' is correct ...

Haven't tried it out with import, only with require, but if your index.js successfully imports the components from the subpaths and then exports them again using a "{}" then I don't see why it wouldn't work?

Maybe it's the difference between import and require ...

@jasan-s
Copy link

jasan-s commented Aug 14, 2017

@leob I get an error using it with import.
index.js file looks as follows:

export  componentA from './componentA/componentA 
export  componentB from './componentB/componentB 

I'll try stack overflow, see if i can get some help there as well.

@will-stone
Copy link

will-stone commented Aug 14, 2017

@jasan-s try

export  {default as componentA} from './componentA/componentA'
export  {default as componentB} from './componentB/componentB'

@jasan-s
Copy link

jasan-s commented Aug 14, 2017

that works :) thanks @will-stone

@leob
Copy link

leob commented Aug 14, 2017

Wow, {default as ...} syntax that's new for me ... like I said I tried it with require only.

@Ehesp
Copy link

Ehesp commented Aug 15, 2017

Don't know about you guys but I much prefer using babel-plugin-root-import:

// .babelrc

{
  "presets": [
    "react-app"
  ],
  "plugins": [
    "babel-plugin-transform-es2015-modules-commonjs",
    ["babel-plugin-root-import", {
      "rootPathSuffix": "src/"
    }]
  ]
}

In any component you can import like so:

// From
import someFile from '../../../../utils/someFile';

// To
import someFile from '~/utils/someFIle';

The ~ acts as your project src/ path. Much cleaner imo, not had any issues with it.

@will-stone
Copy link

@Ehesp I didn't think it was possible to use babel plugins with an un-ejected CRA, if that's the case then this wouldn't be a solution users could add, but maybe something that could be considered by the CRA team to be put directly in to react-scripts? Is it cross platform compatible?

@Ehesp
Copy link

Ehesp commented Aug 15, 2017

@will-stone Ah yes sorry this is an ejected app!

@will-stone
Copy link

Having checked out VueJS for a while, I'm all in favour of using the "@" prefix to denote the src dir.

@alexeyraspopov
Copy link
Contributor

alexeyraspopov commented Oct 10, 2017

I was able to use paths from src folder without dots by adding

NODE_PATH = node_modules:src

to .env file. Cheap and fast solution.

@will-stone
Copy link

@alexeyraspopov is there technically any benefit to using

NODE_ENV = node_modules:src

over

NODE_PATH=src/

?

@alexeyraspopov
Copy link
Contributor

My bad, I apologize, it should be NODE_PATH of course. Edited my comment above.

@pedro-mass
Copy link

I think the question still remains: do we need to add node_modules to the path, or is this done regardless and we can just specify src?

@alexeyraspopov
Copy link
Contributor

A quick test shows that you can just set NODE_PATH = src.

@pedro-mass
Copy link

pedro-mass commented Oct 14, 2017

Was just about to comment that lol. It does break VSCode's cmd+click feature, but I'm fine with that for shorter imports.

@Vanuan
Copy link

Vanuan commented Jan 3, 2018

The issue with symlinking node_modules is that babel will skip transpiling these files by default.

@Satyam
Copy link

Satyam commented Jan 4, 2018

@Vanuan One alternative is to create a folder, say src/vRoots and put all the symlinks into that folder. Since it is not under node_modules Babel will compile its contents. Then you add that folder to NODE_PATH.

As a convention, I started all the symlinks with an underscore, which is otherwise forbidden in NPM so there would be no conflict with npm packages, and it made it clear to the developer that those were virtual folders, not dependencies that could be found in node_modules.

The advantage was that it was meant to test several architectures so I could swap various alternatives by changing the folders the symlinks pointed to, for example, my _connector symlink could point to a regular HTTP client, a WebSockets one or even Firebase. Likewise, on the server side, my _db symlink could point to MySql or MongoDb

Since this folder with the symlinks is along the other source files, exclude it from linting, testing or any other utilities that my browse the folders, otherwise you might get in a loop.

@alexdevero
Copy link

I still can't make this work properly. I have in NODE_PATH=src/ in .env. However, some imports still fail.

This fails:

// Home.jsx
import { Link } from './Links'

This works:

// Home.jsx
import { Link } from './../atoms/Links'

Folder structure:

src/
    app/
        atoms/
            Links.jsx
        pages/
            Home.jsx
        main.jsx
    assets/
```

@jasan-s
Copy link

jasan-s commented Mar 31, 2018

@alexdevero setting NODE_PATH=src/ would like nable you to import from absolute base path like so in home.jsx. import Link from 'app/atoms/Links'

@jasan-s
Copy link

jasan-s commented Mar 31, 2018

@alexdevero setting NODE_PATH=src/ would enable you to import from absolute base path like so in home.jsx. import Link from 'app/atoms/Links'

@alexdevero
Copy link

alexdevero commented Mar 31, 2018

That worked. Thank you very much @jasan-s.

Is it somehow possible, without ejecting, to enable import { Link } from './Links' or import { Link } from 'Links'?

@facebook facebook locked and limited conversation to collaborators Apr 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests