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

re:add src directory to webpack resolve.modules @next #4336

Closed
transitive-bullshit opened this issue Apr 21, 2018 · 6 comments
Closed

re:add src directory to webpack resolve.modules @next #4336

transitive-bullshit opened this issue Apr 21, 2018 · 6 comments
Labels

Comments

@transitive-bullshit
Copy link
Contributor

transitive-bullshit commented Apr 21, 2018

This is meant as a follow-up to #3596, as I'm testing out the newest CRA alpha react-scripts 2.0.0-next.66cc7a90.

The new CRA has solved several of my common reasons for having to use react-app-rewired, but there are a few pain points that remain.

One of them is adding the top-level src to the webpack resolve path so all components can import paths relative to the src root as opposed to relative to the file itself. I find this to be cleaner and more descriptive. E.g., this:

import Foo from 'components/Foo'
import Bar from 'store/Bar'
import Util from 'lib/Util'

instead of

import Foo from '../Foo'
import Bar from '../../store/Bar'
import Util from '../../lib/Util'

I generally accomplish this via react-app-rewired:

module.exports = (config, env) => {
  config.resolve.modules = [
    path.join(__dirname, 'src')
  ].concat(config.resolve.modules)

  return config
}

But I figure this has to be a pretty common practice, and I don't really see any downsides to allowing this in the base CRA webpack config, so I thought I'd suggest it as a very minor change for CRA@next.

I know the current suggestion is to add NODE_PATH=src to .env (#3596) with #1333 tracking a more permanent solution, but as of the latest alpha, this functionality is still not working.

To be clear, I generally add this via react-app-rewired because relying on .env which I've relegated to containing secrets means that the project will no longer "just work" for other devs out of the box.

@transitive-bullshit transitive-bullshit changed the title re:add src directory to webpack resolve.modules re:add src directory to webpack resolve.modules react-scripts@next Apr 21, 2018
@transitive-bullshit transitive-bullshit changed the title re:add src directory to webpack resolve.modules react-scripts@next re:add src directory to webpack resolve.modules @next Apr 21, 2018
@zomars
Copy link

zomars commented Apr 25, 2018

Thank you!

@gaearon
Copy link
Contributor

gaearon commented May 3, 2018

relying on .env which I've relegated to containing secrets

This sounds dangerous. You're aware that any "secrets" used in client-side code will be part of your JS bundle, right? So there shouldn't be anything really sensitive there.

I don't really see any downsides to allowing this in the base CRA webpack config

This differs from Node resolution mechanism. It's a huge downside because then tools can't agree on how to resolve paths.

@gaearon
Copy link
Contributor

gaearon commented May 3, 2018

as of the latest alpha, this functionality is still not working.

Can you elaborate on that? I think at least some support for Workspaces was merged although I don't remember how extensive it is.

@transitive-bullshit
Copy link
Contributor Author

transitive-bullshit commented May 3, 2018

Thanks for the response @gaearon!

This sounds dangerous. You're aware that any "secrets" used in client-side code will be part of your JS bundle, right? So there shouldn't be anything really sensitive there.

Yes, I'm fully aware of this. What I meant was that outside of Facebook-land, you typically have multiple services (like an api sever), each potentially with their own .env, and so treating some .env files as not secret and others as secret is a slippery slope, even though any frontend CRA .env file's contents will of course be made public. This seems like a pedantic argument, though; I'm just not happy with the solution of requiring you to add something to .env in order to get a CRA app that would otherwise work out-of-the-box working.

This differs from Node resolution mechanism. It's a huge downside because then tools can't agree on how to resolve paths.

I can definitely understand this point, although you're not changing the Node resolution mechanism at all. By either appending to NODE_PATH or using webpack's resolve, you're just taking advantage of a built-in override.

as of the latest alpha, this functionality is still not working. Can you elaborate on that? I think at least some support for Workspaces was merged although I don't remember how extensive it is.

Sure -- I just mean that this specific functionality of importing from top-level src/ still isn't working with the latest alpha without one of the aforementioned workarounds.

The second point (changing the resolution mechanism) is the biggest pushback here for sure. If you feel strongly about not disrupting things with this, I understand will close this issue. I do think it's a pretty common and expected style, however, with some clear semantic benefits, though I'm really not sure how to gauge interest on this type of thing aside from letting the issue remain and seeing if people upvote it.

Thanks again!

@stale
Copy link

stale bot commented Nov 2, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs.

@stale stale bot added the stale label Nov 2, 2018
@Timer
Copy link
Contributor

Timer commented Nov 2, 2018

#5118

@Timer Timer closed this as completed Nov 2, 2018
@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants