-
-
Notifications
You must be signed in to change notification settings - Fork 27k
Absolute path guide #1712
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
Closed
Closed
Absolute path guide #1712
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.env is exists in
.gitignore
. So anyone that is collaborating will not have the file and need to do this in the project. I think this setting should not exists in.env
. Can we tell them to install cross-env and docross-env NODE_PATH=./src react-scripts start
in .scripts in package.json instead?@gaearon can we make the NODE_PATH configurable in package.json? 😄 So the user can just set it once and be done with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there was an argument posted somewhere on these issues arguing to check .env into source control, as it doesn't contain any secrets (client side code shouldn't).
I don't believe we're going to add new configuration for this, however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but i believe that .env should contain the environment specific setting like
CHOKIDAR_USE_POLLING
which doesn't need to turned on in specific env. This env variable is making the codes insrc
directly dependent to have it setup.So, this would cause confusion if the docs says it like this right now. For example, when the original author push it to a git repo and other team member tries to clone it, the repo is not working in other team member local machine because of the missing .env files. And we have "it's working on my local machine" problem all over again.
I think right now If we want to document
NODE_PATH
in the docs we need to use the cross-env in the scripts, or make it configurable.The officially supported ways of making an absolute import is discussed here #1065 so I think that this pr would be better to document that instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we'll remove it from gitignore as part of #1344.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok then, I'm not aware of this. Just experienced an "it's working on my local machine" because of
.env
that's is missing. :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why not it doesn't work when I use 'npm start ' to run my project ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall that Rails' dotenv (which takes care of loading this
.env
file in a rails app environment) was able to recognize several.env
files, like.env
,.env.local
,.env.development
,.env.production
, etc. I think this would fix this issue. The settings to be checked into source control live in.env
, where a comment can specify that any other settings needed only by a specific developer in their local environment should be set in the.env.local
file instead.