facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.73k stars 26.85k forks source link

Support absolute paths with packages/ folder #741

Closed gaearon closed 7 years ago

gaearon commented 8 years ago

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:

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?

udfalkso commented 8 years ago

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

https://github.com/timoxley/linklocal

kasperpeulen commented 8 years ago

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 commented 8 years ago

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

amandapouget commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

Please refer to discussion in https://github.com/facebookincubator/create-react-app/issues/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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

terribly at best.

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

AlicanC commented 8 years ago

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 commented 8 years ago

That's my whole app!

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

AlicanC commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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

AlicanC commented 8 years ago

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

You should just treat node_modules as a reserved name.

gaearon commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

I don't like it, but sure.

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

cecilemuller commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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").

kasperpeulen commented 8 years ago

I think it may be a good idea to consider teaching people this pattern:

screen shot 2016-09-28 at 20 06 28

This seem to not only work with create-react-app, but everywhere. You don't need any symlinks or whatever.

amandapouget commented 8 years ago

So, where would your __tests__ folder go, and how would it use imports? For example: src/testHelpers/MyUniqueJestMatchers.js… In banana.tests.js: import MyUniqueJestMatchers from ‘src/testHelpers/MyUniqueJestMatchers’. My point is that not everything that one might need to import is actually best described as a “node_module”. Under your logic, everything in src/ needs to go in src/node_modules at which point you just have two folders for no other reason than to avoid the name collision of having two node_modules in the root directory. Hence, the packages suggestion.

On Sep 28, 2016, at 2:08 PM, Kasper Peulen notifications@github.com wrote:

I think it may be a good idea to consider teaching people this pattern:

https://cloud.githubusercontent.com/assets/1035299/18926166/15048172-85b7-11e6-9021-877437f04477.png This seem to not only work with create-react-app, but everywhere. You don't need any symlinks or whatever.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebookincubator/create-react-app/issues/741#issuecomment-250250336, or mute the thread https://github.com/notifications/unsubscribe-auth/AMQpjHd5Y82onznTIja63FmOmJc4tc0Nks5quq0zgaJpZM4KFvTW.

gaearon commented 8 years ago

I think node_modules, while “working out of the box”, will be confusing to a lot of people and very hard to explain. It’s a cleaner suggestion technically but I just don’t see it as viable/attractive from the usage point of view.

amandapouget commented 8 years ago

Yeah I agree it’s confusing.

On Sep 28, 2016, at 2:58 PM, Dan Abramov notifications@github.com wrote:

I think node_modules, while “working out of the box”, will be confusing to a lot of people and very hard to explain. It’s a cleaner suggestion technically but I just don’t see it as viable/attractive from the usage point of view.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebookincubator/create-react-app/issues/741#issuecomment-250265235, or mute the thread https://github.com/notifications/unsubscribe-auth/AMQpjJJ5WARZLWqgz7zWzo_SE0ZbVFgRks5qurjPgaJpZM4KFvTW.

kasperpeulen commented 8 years ago

Edit: Nevermind, I changed my mind. Maybe going for packages directory is indeed better.

kasperpeulen commented 8 years ago

Am I missing why this would be a bad idea?

@gaearon How about webpack? I just tried this strategy manually in my create-react-app. But won't it be complicated for making sure that webpack understand which node_modules needs to be compiled with babel, and which not? For example, if I add a new directory to packages while I already started npm start, will that work, or would I need to restart the command?

gaearon commented 8 years ago

For example, if I add a new directory to packages while I already started npm start, will that work, or would I need to restart the command?

I want to do this via symlinks so that they also work in editors, and third party tools (e.g. style checkers, storybooks, etc).

xdmx commented 8 years ago

I don't have very much experience with big React applications, and actually not even with production React applications. I've just started to study it recently, so my opinion may not count much.

I personally don't see it as a big deal using src instead of adding an extra packages folder and link src inside of node_modules.

Based on the simple apps I've been developing I'd put an absolute import most of the time, and this way I'd have everything in packages with an almost empty src folder.

I've just created a symlink in node_modules pointing to src and using import Something from 'src/components/Something/' seems a good way to use it. The opt-in would be that you'd need to prefix src, and thus it'd make it much clearer that you're importing from the src folder and not from an external package.

arstin commented 8 years ago

First off, I'm looking forward to this "import from packages" feature! I know it seems to be underway, but just to throw in my little two cents since it gives a slightly different perspective than what was conveyed here.... Perhaps it will be helpful for docs, or simply understanding how it might be used in the wild.

Importing is just about the last thing keeping me from adopting CRA on a real project. As a designer who works in code, I need to be aggressive in keeping code as flexible as is reasonable and in reducing "developer work" (complexity introduced through the realities of implementation). Others in this thread have said that absolute paths work better on mature apps, and that's certainly true. But I think they are equally useful in the earliest stages of a project---especially you carry the responsibility for deciding what to build in addition to how to build it. When a project is still a rough prototype, I'm constantly renaming things, creating new directory structures, moving files around, decomposing the UI in shifting ways, as what the product is gradually comes together. Especially as component directories become more nested, relative paths add a strong but subtle pressure to not make these changes...or at least to not keep the organization of the code in line with design changes. And this, in turn, adds an increasing pressure not to make design changes, or makes it harder to implement them. Find and replace, or just finding where that component is now when importing, is much easier with absolute paths.

When I moved onto webpack almost three years ago I solved this with some hacky globbing script to allow pathless imports. I wouldn't recommend it for CRA (naming doesn't scale, requires more knowledge of the codebase, occasional memory issues in Node, long bootup time), but it has worked well enough for my purposes. The packages idea seems like a nice compromise.

Guria commented 8 years ago

@gaearon

Am I missing why this would be a bad idea?

Poor symlinks support in windows?

gaearon commented 8 years ago

@Guria Do Lerna or pnpm work on Windows? I haven’t investigated this much so if you could share what exactly the limitations are, it would be helpful.

apiep commented 8 years ago

Windows user here. Lerna is working fine on me, just needed to run it with Administrator priviledge

maciejsikora commented 8 years ago

Hi, I have currently big react app with nested directories. Relative paths in such nested structure are tragic approach, i need to think how many ../ i should give in file. Without create-react-app I used babel-root-import. In the past I had been working in PHP environment and relative paths was always anti-pattern, every framework had solution for that. I think packages directory is not a solution because like sad @AlicanC

The problem is that we want to import almost everything using absolute paths

I agree in 100%. I think most of us need global import possibility to change import ../../file to for example ~/app/file.

arstin commented 8 years ago

Am I missing something here?

With babel-root-import you'd write import SomeExample from '~/some/example.js'; with this packages proposal you'd presumably write something like import SomeExample from 'packages/some/example.js';

Isn't the only difference the name of the big directory you stick your code in ("packages" instead of "src" or "~"/root)? If this were my personal setup I'd also use something like babel-root-import or just src for absolute paths, but the reasons given here for using packages in CRA makes sense IMO. I do expect my personal src to only include index.js, and then I'll never think about it again.

I can see static assets getting slightly weird, but I guess you could just have "css" or "images" or "videos" or "assets" packages. Which, come to think of it, sorta makes sense! It's like a media library.

EDIT: I recalled the actual idea was to make "packages" implicit. In fact it's even simpler: some/example.js. And you can just leave out the "~/src/" from babel-root-import.

borisd commented 8 years ago

@gaearon Quick question, this proposal doesn't appear to clash with fixing the current "relative path hell".

Is there a reason not to allow absolute imports from src/ now and consider improvements in the future?

(A lot of new devs are using this excellent project and with every day passing more are learning to use '../../../actions/jinx.js' as "how things work")

tannerlinsley commented 8 years ago

What else needs to be done here @gaearon? (I'd love to help)

adiachenko commented 7 years ago

It's an interesting idea, but it won't work well on Windows. Basically, you'll be forced to constantly run cmd/powershell as Administrator which would be a nightmare in some work environments.

I don't like the semantics of node_modules trick that has been already described above, but when you think about it, it achieves the same result with less hacks. The only difference would be that in one case the explanation would come from CRA documentation while in other you'll be required to understand Node.js a little bit.

Fofir commented 7 years ago

Might it be possible to utilise Webpack's resolve-aliases feature for this particular case?

I have been using it in recent projects for exact that reason (absolute paths)

FFX01 commented 7 years ago

I don't know if this solves the problem in the way that it needs to be solved, but I have my own approach that seems to be much simpler.

I usually just add the path to the webpack config aliases. Say I have a folder structure like the following:

> webpack.config.js
+ src/
    > index.js
    > App.jsx
    + store/
        > index.js
        > actions.js
    + components/
        > ...?
        + NestedComponents
            > ...?

It's not a perfect example, I know. You can set up your aliases in your webpack config like this:

aliases: {
    '@store': path.resolve('src/store'),
    '@components': path.resolve('src/components')
}

Then in your code you can do this:

~/App.jsx

import store from '@store'
import Component from '@components'
import AnotherComponent from '@components/NestedComponents'

Is there any reason a standardized system similar to the one presented here would not work?

gaearon commented 7 years ago

I'm coming to the conclusion that the best way forward is to just document the src/node_modules "hack". It's compatible with all the tooling. We just need to fix small issues like #1042.

gaearon commented 7 years ago

Superseded by #1065.

leob commented 7 years ago

Thanks @mandysimon88 for putting me on the right track with your suggestion:

NODE_PATH=src/ && npm run start

However it did not work for me till I changed it to:

NODE_PATH=src/ npm run start

Ah, I see that the first variant is Windows shell syntax and the latter variant is Unix/Linux ... I think there are utilities (npm packages and so on) to hide these differences.

Anyway I could then get rid of relative import paths in my app (which are quite terrible indeed) without having to change the Webpack config (which create-react-app does not support unless you "eject").

will-stone commented 7 years ago

Glad I stumbled upon this. I've put NODE_PATH=src/ in my .env file and now I can use this import Button from 'components/Button' from any file. Thank-you all.

kellyrmilligan commented 7 years ago

just fyi if using flow,

[options]
module.system.node.resolve_dirname=node_modules
module.system.node.resolve_dirname=src

to map it the same way.