-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Add pragmatic advice on file structure #338
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
Conversation
Deploy preview ready! Built with commit 71788c6 |
One common way to structure projects is locate CSS, JS, and tests together inside folders grouped by feature or route. | ||
|
||
``` | ||
FeatureA | ||
common/ | ||
Avatar.js |
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.
Any chance I could sway things from PascalCase to kebab-case? I've seen many people (especially beginners) have issues with case-sensitivity across operating systems and with git. I've found it best to avoid these issues altogether by encouraging all lower case and using kebab-case to separate words. I'd love it if the recommended approach in React's docs helped encourage this as well to avoid casing issues for people following the suggestion.
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 think that's going to be a hard sell for most people since it doesn't match component names.
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.
Interesting! I've haven't ran into case-sensitivity issues in my own development yet, but this does sound like a very real concern. I would support using kebab-case, and maybe even explaining why it is recommended over PascalCase.
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 think kebab-case is a little less intuitive on how to import without opening the file up, i.e.
is my-component
imported as MyComponent
, myComponent
, or mycomponent
I think if you maintain the casing and the component name to match, it makes it easier to not constantly reference the file.
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.
Kebab-case is for npm packages. PascalCase for local components. My 0.02€.
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 fairly regularly see Gatsby issues where people can't deploy from linux CI because of casing issues not discovered on their mac.
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.
@kentcdodds One viable alternative you could do is encourage ES Lint.
We won't get into the savagery of eslint-config-airbnb
, but I think you could recommend import/no-unresolved
which throws error when the import declaration doesn't match the underlying filesystem.
Anomalies can only persist for seconds.
I would rather see people adopting ES Lint and using its built in "recommendation engine" to not only stop time waste errors but also to skill up further by researching why some rules pop up. I consider Airbnb as incredible for intermediate-level wizards, assuming one researches and comes to understand why each rule exists. Bottom line, I think it is a pathway to improve knowledge of JavaScript itself.
Gave it a read through and it all sounds good to me! Tried looking for big spelling/grammar mistakes too and nothing popped out. 👍 |
content/docs/faq-structure.md
Outdated
ProfileAPI.js | ||
``` | ||
|
||
The definition of what a "feature" is not universal, and it is up to you to choose the granularity. If you can't come up with a list of top-level folders, you can ask the users of your product what major parts it consists of, and use their mental model as a blueprint. |
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.
There's an "is" missing here -> "The definition of what a feature is is not universal"
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.
The definition of what a "feature" is not universal
The definition of what a "feature" can be is not universal ?
LGTM 💯 |
content/docs/faq-structure.md
Outdated
ProfileAPI.js | ||
``` | ||
|
||
The definition of what a "feature" is not universal, and it is up to you to choose the granularity. If you can't come up with a list of top-level folders, you can ask the users of your product what major parts it consists of, and use their mental model as a blueprint. |
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.
What will index.js
consist of in this case?
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.
💯
|
||
#### Avoid too much nesting | ||
|
||
There are many pain points associated with deep directory nesting in JavaScript projects. It becomes harder to write relative imports between them, or to update those imports when the files are moved. Unless you have a very compelling reason to use a deep folder structure, consider limiting yourself to a maximum of three or four nested folders within a single project. Of course, this is only a recommendation, and it may not be relevant to your 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.
What about for those using absolute paths?
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.
That's exactly the kind of detail I don't want to get into :-) If you know how to set this up and understand the tradeoffs of using symlinks vs non-standard setup that doesn't work with other tooling, you already know more than this section can give you.
Sync with reactjs.org @ 8112446
…md (reactjs#338) * Update translate Just change the translation as original confuses me. * Update higher-order-components.md * Update higher-order-components.md
Figured I'd write this