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

Document src/node_modules as official solution for absolute imports #1065

Closed
4 tasks
gaearon opened this issue Nov 20, 2016 · 30 comments
Closed
4 tasks

Document src/node_modules as official solution for absolute imports #1065

gaearon opened this issue Nov 20, 2016 · 30 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Nov 20, 2016

After a long discussion in #741 it seems to me that src/node_modules seems like the best solution for absolute imports.

It has the following benefits:

  • Doesn't rely on symlinks (they don't work well on Windows).
  • Works with IDEs and any existing tooling because it relies on Node resolution mechanism.
  • If you really want to, you can run ln -s src/node_modules src/packages and use the "nicer" packages (or any other) folder.

This issue is a call to help to make this approach the official one. We need to:

If you want to work on either please leave a comment and then submit a PR!
Cheers.

@fadeojo
Copy link

fadeojo commented Nov 21, 2016

I want to help but I will need someone to shadow.

@gaearon
Copy link
Contributor Author

gaearon commented Nov 21, 2016

Feel free to jump on either of those issues and let me know if you need any help!

@tbranyen
Copy link

This is a very interesting idea! I'm wondering though if it'd make more sense to support a top-level packages directory (exactly like lerna) and symlink that to src/node_modules. This binding could exist for all create-react-apps.

src/
src/node_modules -> ../packages
packages/
public/

The reasoning for this is that typically your external modules are general purpose and not application-specific (therefore not desirable in the application source directory), and to support something like lerna. Maybe you want to do code splitting via npm packages? This would allow you to support lerna and create-react-app in the same project.

For selfish reasons, this would allow me to use create-react-app to kitchen sink/playground a bunch of lerna components.

@viankakrisna
Copy link
Contributor

what's the issue with cross-env NODE_PATH=./src react-scripts start ? I think it's cleaner and more clear rather than having a node_modules inside src folder.

@gaearon
Copy link
Contributor Author

gaearon commented Nov 23, 2016

Most people don't know about cross-env so they just write commands that don't work on Windows. This hurts open-source examples (e.g. people can't run a React example because of its reliance on Bash environment variable syntax). NODE_PATH is also considered legacy and slightly discouraged so it doesn’t feel right to promote this as a solution. Just using src as the root can also be a bit surprising, and the explicitness of node_modules has some benefits (e.g. you know for sure they’ll have priority over regular node_modules).

Ultimately I just want both ways to work, and we can document them. People can then choose whichever way they prefer.

@gaearon
Copy link
Contributor Author

gaearon commented Nov 23, 2016

support a top-level packages directory (exactly like lerna) and symlink that to src/node_modules

I believe this is what I suggested in the opening comment (but only as opt-in):

If you really want to, you can run ln -s src/node_modules src/packages and use the "nicer" packages (or any other) folder.

It has to be opt-in though, please read the discussion about Windows issues in #741.

@daltones
Copy link

I personally like the idea of a src/node_modules, just because it's absolutely free of hacks.

But right now there's some problems with linters:

@kompot
Copy link

kompot commented Nov 29, 2016

Another way of managing absolute paths that might have come up but could not find it in
#741

We've been using wix's wml
https://github.com/wix/wml
to get any required project structure with:

  • absolute paths
  • no symlinks

As project's readme states — they just use watchman to watch for certain dirs and copy files to node_modules subfolders. We've added some additional scripts to automate bootstrapping but it turned out to work really smooth.

Sure it's not an ideal solution but it seems to work really nice and does not force you to follow any conventions so it's easily can be applied to any project.

What do you think?

@AdamTyler
Copy link

I've created a link src/node_modules to src/common and this allows me to import a component using import Rating from 'components/Rating' (components is a folder in common).

However, when I run my tests, jest gives me the following error:

Test suite failed to run

    SyntaxError: Unexpected reserved word

on the line where I am doing that import.

Is there something I need to add to my jest config to make this work properly?

@gaearon
Copy link
Contributor Author

gaearon commented Dec 1, 2016

@AdamTyler

Please see the first post in this thread 😉

screen shot 2016-12-01 at 17 49 42

@gaearon gaearon modified the milestones: 0.11.0, 0.10.0 May 8, 2017
@Janpot
Copy link
Contributor

Janpot commented May 14, 2017

will there also be a way to build and npm publish these as separate components?

@azz
Copy link

azz commented May 14, 2017

@Janpot I'd look at a tool like lerna if you're publishing multiple packages from one repo.

@Janpot
Copy link
Contributor

Janpot commented May 14, 2017

@azz That's not really the reason why I ask this question. I mean adding what would be the component equivalent of create-react-app. "How to create a reusable react component with no build configuration?". The "standard way" let's say, to create and distribute components. When I look around the landscape, some components are ES2015 bundles, some have external CSS, some have styles bundled with a style loader, some create UMD bundles and duplicate all my dependencies,... Each have their own flavour of build configuration. It's annoying and counter-productive to figure out this toolchain each time you try to contribute to a repo. a solution like create-react-app that is focused on components could be very helpful here. It might not cover 100% of the use cases, but neither does create-react-app.

@Janpot
Copy link
Contributor

Janpot commented May 14, 2017

@viankakrisna Indeed, sort of the create-react-app equivalent of that nwb feature I guess.

@sidoshi
Copy link
Contributor

sidoshi commented May 20, 2017

If I understand correctly, the only reason we are not using NODE_PATH env by default is because it would conflict with node_modules. But it's much cleaner than adding node_modules in src and configuring jest to ignore node_modules inside src.Instead, we could use NODE_PATH=src by default with cross environment support and document it properly that the modules in src have preference over node_modules.

Further, we could enforce the direct folders or files inside src be in PascalCase and as npm doesn't allow package names to start with capital letters,

import React from 'React'

won't conflict with the react in node_modules and the user will know that it is an absolute import from src.

@tbranyen
Copy link

@doshisid Wouldn't that only work in case sensitive filesystems? If so, 👎

@sidoshi
Copy link
Contributor

sidoshi commented May 20, 2017

We could find a common case for all filesystems and enforce that. Like maybe prefixing with _

@shendepu
Copy link

shendepu commented Jun 15, 2017

For the absolute import, beside of packages approach which is good pattern to split code into packages, why not simply put the src as the first one for webpack resolve.modules?

In the fractal project structure

src
-- components
-- routes
    -- jobs
      -- components
      -- actions
      -- list
        -- components
        -- actions
      -- detail
        -- components
        -- actions

for code in src/routes/jobs/details/actions to import something from src/routes/jobs/actions, it still needs to use ../../actions to relative import. This is simple case, with nest structure getting deeper, it will be something like ../../../../actions which is hard to read what is imported. With src as root resolved, it is crystal to say import from src/routes/jobs/actions

And with packages approach, even routes is package under packages folder, this long back path relative import is still inevitable.

=============== UDATE: ===============
Find it is simple to add NODE_PATH=src in .env file so that webpack modules.resolve will have src. even better to add NODE_PATH=src:packages, then no npm link need for packages in packages folder

@Danilo-Araujo-Silva
Copy link

Danilo-Araujo-Silva commented Jun 27, 2017

For me using node_modules inside src is not beeing a option because jest is crashing. Very annoying.

So my solution was using NODE_PATH=src in my package.json and in WebStorm/Intellij I simply did:

Right click on src folder -> Mark Directory as -> Sources Root

Voi lá! Everything is working fine! (go to files, open classes, moving files to another directory automatically change the references)

A note about NODE_PATH is that in the current docs (Node 8.1.2) is not saying that NODE_PATH is legacy or will be deprecated but that it is not used as it was used before:

NODE_PATH is still supported, but is less necessary now that the Node.js ecosystem has settled on a convention for locating dependent modules. Sometimes deployments that rely on NODE_PATH show surprising behavior when people are unaware that NODE_PATH must be set. Sometimes a module's dependencies change, causing a different version (or even a different module) to be loaded as the NODE_PATH is searched.

@nazreen
Copy link

nazreen commented Jul 1, 2017

the docs quote above convinces me that it's not deprecated, contrary to what OP has implied.
I'm a 'new' developer myself and I feel like this node_modules solution will be confusing or seem strange to some. the alternative of using NODE_PATH seems like the better way.

@cecilemuller
Copy link

cecilemuller commented Jul 1, 2017

NODE_PATH require an extra dependency to be cross-platform, tools has to specify the extra variable, and it works here only because it's the toplevel (as the react app isn't a module that can be imported in another module), otherwise it would introduce extra complexity there too.

But pick whatever you prefer: I'm not fond of /src/node_modules either, just is the most pragmatic in the end given the way Node is at the moment.

@bmac
Copy link

bmac commented Jul 10, 2017

I tried playing around with using this approach in my project and I noticed react-scripts start wasn't picking up changes inside src/node_modules because it was ignoring node_modules. I opened #2760 to update the dev server config to play nicely with src/node_modules.

@icopp
Copy link

icopp commented Sep 22, 2017

For absolute imports, it seems like it'd be a lot better to have some kind of special prefix and set up webpack's alias functionality for it. For example, my personal projects use:

{
  resolve: {
    alias: {
      "@": path.resolve(__dirname, 'src')
    }
  }
}

...after which you can reference files via...

import App from '@/App' // the file is at `src/App/index.jsx`

You can also inject the same schema into Jest:

"moduleNameMapper": {
  "^@/(.*)$": "<rootDir>/src/$1"
}

It keeps a clear distinction from npm-tracked node_modules, avoiding the "I can't get it to work and I don't understand why" potential of having overlapping names or having imports start with just ./.

benoccrp added a commit to occrp-attic/aleph-ui that referenced this issue Nov 30, 2017
Solution using .env to set NODE_PATH (commit c148630) appears unreliable.

Discussions about absolute imports for create-react-app are ongoing;
making a symlink seems the currently recommended approach. I call it
'src' for clarity.
See e.g. facebook/create-react-app#1065
@gaearon
Copy link
Contributor Author

gaearon commented Jan 8, 2018

Closing in favor of #1333.
src/node_modules is too weird to most people and not very ergonomic.

There's been a few underlying issues with Jest which have been fixed so we can take another shot at this.

@bjankord
Copy link

bjankord commented Oct 1, 2018

Sorry to resurrect an old issue, though with #1333 being not quite ready, is src/node_modules a viable option for people to wanting aboslute imports @gaearon? I like the idea that it relies on Node resolution mechanism, but wasn't sure if this approach was discouraged.

@lock lock bot locked and limited conversation to collaborators Jan 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests