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

Application Structure #27

Closed
mxstbr opened this Issue Dec 17, 2015 · 76 comments

Comments

Projects
None yet
@mxstbr
Member

mxstbr commented Dec 17, 2015

Changing the application structure might be worth thinking about, right now the thing might seem quite bloated and randomly thrown together for newcomers.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 18, 2015

Member

Maybe we should have a folder called app, which is the folder you write your application in and only contains the application files and folders, and a folder config (or webpack?) for all the webpack stuff. This would allow us to tell developers to simply write their application in the app folder and write tests in the test folder and don't care about the rest. (should the test folder be inside the app folder?)

The structure of the boilerplate would then be as follows:

app/
|   js/
|   css/
|   img/    <--- maybe rename this to "assets"
|   .htaccess
|   index.html
|   manifest.json
|   serviceworker.js
config/    <--- maybe this should be called "webpack"
|   makewebpackconfig.js
|   server.js
|   webpack.dev.config.js
|   webpack.prod.config.js
docs/
|   COMMANDS.md
|   FILES.md
|   README.md
|   moretocomehere
test/    <--- maybe rename this to "tests"
|   reducers/
|   actions/
.babelrc
.eslintignore
.eslintrc
.gitattributes
.gitignore
LICENSE.md
README.md
package.json
Member

mxstbr commented Dec 18, 2015

Maybe we should have a folder called app, which is the folder you write your application in and only contains the application files and folders, and a folder config (or webpack?) for all the webpack stuff. This would allow us to tell developers to simply write their application in the app folder and write tests in the test folder and don't care about the rest. (should the test folder be inside the app folder?)

The structure of the boilerplate would then be as follows:

app/
|   js/
|   css/
|   img/    <--- maybe rename this to "assets"
|   .htaccess
|   index.html
|   manifest.json
|   serviceworker.js
config/    <--- maybe this should be called "webpack"
|   makewebpackconfig.js
|   server.js
|   webpack.dev.config.js
|   webpack.prod.config.js
docs/
|   COMMANDS.md
|   FILES.md
|   README.md
|   moretocomehere
test/    <--- maybe rename this to "tests"
|   reducers/
|   actions/
.babelrc
.eslintignore
.eslintrc
.gitattributes
.gitignore
LICENSE.md
README.md
package.json

@mxstbr mxstbr changed the title from Moving all files possible to src folder to Application Structure Dec 18, 2015

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 18, 2015

Member

I believe having the test (tests) folder inside the app folder makes sense. That way the separation of code-the-user-has-to-write and tooling is complete, and we can tell users they should only care about the app folder and let the boilerplate handle the rest.

The more I think about this structure, the more I like it. It makes using this boilerplate much easier and newcomer friendly. The structure would look like this:

app/
|   js/
|   css/
|   img/    <--- maybe rename this to "assets"
|   test/    <--- maybe rename this to "tests"
|   |   reducers/
|   |   actions/
|   .htaccess
|   index.html
|   manifest.json
|   serviceworker.js
config/    <--- maybe this should be called "webpack"
|   makewebpackconfig.js
|   server.js
|   webpack.dev.config.js
|   webpack.prod.config.js
docs/
|   COMMANDS.md
|   FILES.md
|   README.md
|   moretocomehere
.babelrc
.eslintignore
.eslintrc
.gitattributes
.gitignore
LICENSE.md
README.md
package.json
Member

mxstbr commented Dec 18, 2015

I believe having the test (tests) folder inside the app folder makes sense. That way the separation of code-the-user-has-to-write and tooling is complete, and we can tell users they should only care about the app folder and let the boilerplate handle the rest.

The more I think about this structure, the more I like it. It makes using this boilerplate much easier and newcomer friendly. The structure would look like this:

app/
|   js/
|   css/
|   img/    <--- maybe rename this to "assets"
|   test/    <--- maybe rename this to "tests"
|   |   reducers/
|   |   actions/
|   .htaccess
|   index.html
|   manifest.json
|   serviceworker.js
config/    <--- maybe this should be called "webpack"
|   makewebpackconfig.js
|   server.js
|   webpack.dev.config.js
|   webpack.prod.config.js
docs/
|   COMMANDS.md
|   FILES.md
|   README.md
|   moretocomehere
.babelrc
.eslintignore
.eslintrc
.gitattributes
.gitignore
LICENSE.md
README.md
package.json
@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 18, 2015

Member

Maybe config is the wrong name for the folder with boilerplate things. That might lead devs to think that they can configure their application in there, which the can't.

Maybe tooling is a better name? I don't want to call it webpack, because there might be other config we want to put somewhere later on.

Member

mxstbr commented Dec 18, 2015

Maybe config is the wrong name for the folder with boilerplate things. That might lead devs to think that they can configure their application in there, which the can't.

Maybe tooling is a better name? I don't want to call it webpack, because there might be other config we want to put somewhere later on.

@swastik

This comment has been minimized.

Show comment
Hide comment
@swastik

swastik Dec 21, 2015

I think calling it webpack is a good idea because it's very clear. tooling is somewhat ambiguous. You can always change the name later.

img should probably be changed to assets, yeah, because you could add webfonts, etc. that way.

swastik commented Dec 21, 2015

I think calling it webpack is a good idea because it's very clear. tooling is somewhat ambiguous. You can always change the name later.

img should probably be changed to assets, yeah, because you could add webfonts, etc. that way.

@swastik

This comment has been minimized.

Show comment
Hide comment
@swastik

swastik Dec 21, 2015

Also, I don't think tests should go inside app. They aren't the "app", technically... nothing there would transpile to the final build. A separate tests folder is fine... it's also probably more visible.

swastik commented Dec 21, 2015

Also, I don't think tests should go inside app. They aren't the "app", technically... nothing there would transpile to the final build. A separate tests folder is fine... it's also probably more visible.

mxstbr added a commit that referenced this issue Dec 22, 2015

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 22, 2015

Member

Good points. I've implemented this in the v3.0.0 branch, check it out and report any problems you come across!

EDIT: Going to update the docs/FILES.md file soon.

Member

mxstbr commented Dec 22, 2015

Good points. I've implemented this in the v3.0.0 branch, check it out and report any problems you come across!

EDIT: Going to update the docs/FILES.md file soon.

@swastik

This comment has been minimized.

Show comment
Hide comment
@swastik

swastik Dec 23, 2015

Looks pretty good!

swastik commented Dec 23, 2015

Looks pretty good!

@mxstbr mxstbr added this to the v3.0 milestone Dec 24, 2015

@mxstbr mxstbr self-assigned this Dec 25, 2015

@hampusohlsson

This comment has been minimized.

Show comment
Hide comment
@hampusohlsson

hampusohlsson Dec 26, 2015

You could consider bundling the actions, constants and reducers into redux modules and create reducer bundles according to the "ducks" approach.

hampusohlsson commented Dec 26, 2015

You could consider bundling the actions, constants and reducers into redux modules and create reducer bundles according to the "ducks" approach.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 27, 2015

Member

As discussed in #33, having a folder for "smart" components and one for "dumb" components makes a lot of sense. The question is where do pages fit in?

I currently feel like this would be optimal - any other ideas?

app/
|--- js/
    |--- containers/    <--- stateful components
         |--- pages/
    |--- components/ <--- stateless components
Member

mxstbr commented Dec 27, 2015

As discussed in #33, having a folder for "smart" components and one for "dumb" components makes a lot of sense. The question is where do pages fit in?

I currently feel like this would be optimal - any other ideas?

app/
|--- js/
    |--- containers/    <--- stateful components
         |--- pages/
    |--- components/ <--- stateless components
@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 27, 2015

Member

@hampusohlsson thanks for the link, I'll look into it!

Member

mxstbr commented Dec 27, 2015

@hampusohlsson thanks for the link, I'll look into it!

@mxstbr mxstbr referenced this issue Dec 27, 2015

Closed

CSS Structure #32

@hampusohlsson

This comment has been minimized.

Show comment
Hide comment
@hampusohlsson

hampusohlsson Dec 27, 2015

You can probably put it in a subfolder if you need it. However, my (rather limited) experience with this structure is that page components end up being the only smart ones in the entire app, so you can just keep them under containers to remove the extra level

hampusohlsson commented Dec 27, 2015

You can probably put it in a subfolder if you need it. However, my (rather limited) experience with this structure is that page components end up being the only smart ones in the entire app, so you can just keep them under containers to remove the extra level

@philihp

This comment has been minimized.

Show comment
Hide comment
@philihp

philihp Dec 28, 2015

Contributor

I would actually be in favor of putting css and javascript. For example:

app/
    components/
               clicker.js
               clicker.css
               switcher.js
               switcher.css

clicker.css is so intimately related to clicker.js that changes to one made by someone new to the codebase should probably want to check out the other file.

Since your build is a webpack bundle, it would also be cool to do this with tests as well. That would alleviate the need to have a tests/ folder that tries to mirror the structure. Most projects don't do this because they transpile the src/ folder, and you don't want that to include tests. Webpack is smarter about this.

Contributor

philihp commented Dec 28, 2015

I would actually be in favor of putting css and javascript. For example:

app/
    components/
               clicker.js
               clicker.css
               switcher.js
               switcher.css

clicker.css is so intimately related to clicker.js that changes to one made by someone new to the codebase should probably want to check out the other file.

Since your build is a webpack bundle, it would also be cool to do this with tests as well. That would alleviate the need to have a tests/ folder that tries to mirror the structure. Most projects don't do this because they transpile the src/ folder, and you don't want that to include tests. Webpack is smarter about this.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 28, 2015

Member

I'm considering switching to this layout, as it makes a lot of sense for bigger applications. No tests/ and no css/ folders anymore:

app/
|- assets/
|- js/
|  |- components/
|    |- NavBar/
|      |- NavBar.react.js
|      |- NavBar.css
|      |- NavBar.test.js
|    |- Button/
|      |- Button.react.js
|      |- Button.css
|      |- Button.test.js
|  |- containers/
|    |- HomePage/
|      |- HomePage.react.js
|      |- HomePage.css
|      |- HomePage.test.js
|- .htaccess
|- index.html
|- manifest.json
|- serviceworker.js

With CSS Modules importing the CSS files of each component in the .react.js file. Webpack's css-loader (which we use anyway) has support for CSS modules, so it wouldn't be hard to switch.

Where do vendor CSS files and global CSS files (e.g. variables.css, typography.css,...) now fit in?

Member

mxstbr commented Dec 28, 2015

I'm considering switching to this layout, as it makes a lot of sense for bigger applications. No tests/ and no css/ folders anymore:

app/
|- assets/
|- js/
|  |- components/
|    |- NavBar/
|      |- NavBar.react.js
|      |- NavBar.css
|      |- NavBar.test.js
|    |- Button/
|      |- Button.react.js
|      |- Button.css
|      |- Button.test.js
|  |- containers/
|    |- HomePage/
|      |- HomePage.react.js
|      |- HomePage.css
|      |- HomePage.test.js
|- .htaccess
|- index.html
|- manifest.json
|- serviceworker.js

With CSS Modules importing the CSS files of each component in the .react.js file. Webpack's css-loader (which we use anyway) has support for CSS modules, so it wouldn't be hard to switch.

Where do vendor CSS files and global CSS files (e.g. variables.css, typography.css,...) now fit in?

@zavelevsky

This comment has been minimized.

Show comment
Hide comment
@zavelevsky

zavelevsky Dec 28, 2015

in that case - 'js' is not a good name for the parent folder 😄
Also - why not 'jsx' extensions for files containing jsx? Looks nicer than '.react.js'.

zavelevsky commented Dec 28, 2015

in that case - 'js' is not a good name for the parent folder 😄
Also - why not 'jsx' extensions for files containing jsx? Looks nicer than '.react.js'.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 28, 2015

Member

True - any ideas for a better name for the parent folder?

To be honest, I can't remember why I adopted that naming. Submitted a new issue for this, see #50

Member

mxstbr commented Dec 28, 2015

True - any ideas for a better name for the parent folder?

To be honest, I can't remember why I adopted that naming. Submitted a new issue for this, see #50

@zavelevsky

This comment has been minimized.

Show comment
Hide comment
@zavelevsky

zavelevsky Dec 28, 2015

This is my project structure. Hopes it helps.
Whenever you see a file with .dev. in its name you know I alternate dev/production version of this file using NormalModuleReplacementPlugin.

.
├── build (bundled files for production server)
├── lib
│   └── dev_server.js
├── package.json
├── src
│   ├── fonts (I like to provide everything from my server since it's better for on-premise installations)
│   ├── images
│   │   ├── app_logo.png
│   │  └── favicon.ico
│   ├── js
│   │   ├── api (server communication. 
│   │   │   ├── MOCK_DATA.js
│   │   │   ├── WebApi.dev.js (NormalModuleReplacementPlugin loads this on dev build - so I can get mock data for my "server calls"
│   │   │   └── WebApi.js
│   │   ├── constants
│   │   │   └── AppConstants.js
│   │   ├── core (wraps the state object)
│   │   │   ├── consts.js
│   │   │   └── core.js
│   │   ├── react
│   │   │   ├── components
│   │   │   │   ├── DateSpan.jsx
│   │   │   │   └── VisibilityFilter.jsx
│   │   │   ├── containers
│   │   │   │   ├── App.jsx (using Root helps keeping the App clean)
│   │   │   │   ├── DevTools.jsx
│   │   │   │   ├── Root.dev.jsx (split to dev and prod for different redux store initialization)
│   │   │   │   ├── Root.jsx
│   │   │   │   └── GalleryPage.jsx
│   │   │   ├── helpers
│   │   │   │   ├── canvas.js
│   │   │   │   └── state.js
│   │   │   └── main.jsx (entry point)
│   │   ├── redux
│   │   │   ├── actions
│   │   │   ├── constants.js
│   │   │   ├── helpers
│   │   │   │   └── create_reducer.js
│   │   │   ├── middleware
│   │   │   ├── persistence
│   │   │   ├── reducers
│   │   │   └── store
│   │   │       ├── configureStore.dev.js
│   │   │       └── configureStore.js
│   │   ├── routing
│   │   │   ├── constants.dev.js
│   │   │   ├── constants.js
│   │   │   ├── history.js
│   │   │   └── utils.js
│   │   └── utils
│   │       ├── JsonUtils.js
│   │       └── interval.js
│   ├── styles
│   └── templates
│       └── main_template.html
├── test
└── webpack.config.js


zavelevsky commented Dec 28, 2015

This is my project structure. Hopes it helps.
Whenever you see a file with .dev. in its name you know I alternate dev/production version of this file using NormalModuleReplacementPlugin.

.
├── build (bundled files for production server)
├── lib
│   └── dev_server.js
├── package.json
├── src
│   ├── fonts (I like to provide everything from my server since it's better for on-premise installations)
│   ├── images
│   │   ├── app_logo.png
│   │  └── favicon.ico
│   ├── js
│   │   ├── api (server communication. 
│   │   │   ├── MOCK_DATA.js
│   │   │   ├── WebApi.dev.js (NormalModuleReplacementPlugin loads this on dev build - so I can get mock data for my "server calls"
│   │   │   └── WebApi.js
│   │   ├── constants
│   │   │   └── AppConstants.js
│   │   ├── core (wraps the state object)
│   │   │   ├── consts.js
│   │   │   └── core.js
│   │   ├── react
│   │   │   ├── components
│   │   │   │   ├── DateSpan.jsx
│   │   │   │   └── VisibilityFilter.jsx
│   │   │   ├── containers
│   │   │   │   ├── App.jsx (using Root helps keeping the App clean)
│   │   │   │   ├── DevTools.jsx
│   │   │   │   ├── Root.dev.jsx (split to dev and prod for different redux store initialization)
│   │   │   │   ├── Root.jsx
│   │   │   │   └── GalleryPage.jsx
│   │   │   ├── helpers
│   │   │   │   ├── canvas.js
│   │   │   │   └── state.js
│   │   │   └── main.jsx (entry point)
│   │   ├── redux
│   │   │   ├── actions
│   │   │   ├── constants.js
│   │   │   ├── helpers
│   │   │   │   └── create_reducer.js
│   │   │   ├── middleware
│   │   │   ├── persistence
│   │   │   ├── reducers
│   │   │   └── store
│   │   │       ├── configureStore.dev.js
│   │   │       └── configureStore.js
│   │   ├── routing
│   │   │   ├── constants.dev.js
│   │   │   ├── constants.js
│   │   │   ├── history.js
│   │   │   └── utils.js
│   │   └── utils
│   │       ├── JsonUtils.js
│   │       └── interval.js
│   ├── styles
│   └── templates
│       └── main_template.html
├── test
└── webpack.config.js


@hampusohlsson

This comment has been minimized.

Show comment
Hide comment
@hampusohlsson

hampusohlsson Dec 28, 2015

As image assets also are part of the "source", I usually make one directory called src that contains everything that will be processed by webpack. This is my own preference, but I like to name the js-files inside the components just index.js because that allows you to do something like this from another component:

// Importing a button from the IndexPage would look like this
import Button from '../../components/Button' 

and inside Button/index.js you would import the styles

// Include the styles
import styles from './style.scss' 

Images would obvioulsy have to be imported from the assets folder

import Icon from '../../assets/images/icon.png'

Here's an example of how I've structured one project (a bunch of stuff omitted for brevity)

├── dist
│   ├── css <-- webpack extracted css goes here
│   ├── img <-- webpack processed images goes here
│   └── js <-- webpack compiled js goes here
└── src
    ├── assets
    │   ├── images
    │   └── scss <-- global css such as vendor, variables and typography
    ├── components
    │   ├── Button
    │   │   ├── index.js
    │   │   └── style.scss
    │   └── Table
    ├── containers
    │   └── IndexPage
    └── redux <-- ducks goes in here

In the same manner, it would make a lot of sense to just add a file called test.js for every component. This way, we create a standardized way of how every component should look while keeping it simple. The name of the component is the folder name, which contains an index.js file, a style.css and a test.js file.

Re the global css; right now I'm combining the scss files that contains overall layout, variables and typography into one scss file called base.scss which I'm requiring in the file where I define my routes.

hampusohlsson commented Dec 28, 2015

As image assets also are part of the "source", I usually make one directory called src that contains everything that will be processed by webpack. This is my own preference, but I like to name the js-files inside the components just index.js because that allows you to do something like this from another component:

// Importing a button from the IndexPage would look like this
import Button from '../../components/Button' 

and inside Button/index.js you would import the styles

// Include the styles
import styles from './style.scss' 

Images would obvioulsy have to be imported from the assets folder

import Icon from '../../assets/images/icon.png'

Here's an example of how I've structured one project (a bunch of stuff omitted for brevity)

├── dist
│   ├── css <-- webpack extracted css goes here
│   ├── img <-- webpack processed images goes here
│   └── js <-- webpack compiled js goes here
└── src
    ├── assets
    │   ├── images
    │   └── scss <-- global css such as vendor, variables and typography
    ├── components
    │   ├── Button
    │   │   ├── index.js
    │   │   └── style.scss
    │   └── Table
    ├── containers
    │   └── IndexPage
    └── redux <-- ducks goes in here

In the same manner, it would make a lot of sense to just add a file called test.js for every component. This way, we create a standardized way of how every component should look while keeping it simple. The name of the component is the folder name, which contains an index.js file, a style.css and a test.js file.

Re the global css; right now I'm combining the scss files that contains overall layout, variables and typography into one scss file called base.scss which I'm requiring in the file where I define my routes.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 28, 2015

Member

@zavelevsky that's very helpful, thanks! What do the constants.js in the routing folder do?

@hampusohlsson We have the app/ folder, which is like your src/ folder, and the build/ folder, which is like your dist/ folder. 👍
I've thought about doing the index.js thing, but it's non-obvious for beginners that it gets exported by default. We have a lot of magic happening already, so I'd rather leave it as it is for now.
I agree with the tests next to the components, that sounds good! It's dependent on #11 though, I have yet to find a nice way to unit test components... (@philihp pinged me on twitter about airbnb/enzyme which looks really nice. I hope he tries implementing it!)

This is what I'm currently looking at:

app/
|- assets/
|  |- img/
|  |- fonts/
|- css/    <--- Global CSS stuff, variables, mixins etc.
|  |- vendor/
|  |- utils/
|  |- main.css    <--- Main CSS file that imports everything
|- js/
|  |- actions/
|    |- AppActions.js    <--- Global Actions
|  |- constants/
|    |- AppConstants.js/    <--- Global Constants
|  |- components/    <--- Stateless components
|    |- NavBar/
|      |- NavBar.react.js     |
|      |- NavBar.css          | <--- Each stateless component has a .css file, a .js file and a .test.js file
|      |- NavBar.test.js      |
|    |- Button/
|      |- Button.react.js
|      |- Button.css
|      |- Button.test.js
|  |- containers/    <--- Stateful components
|    |- HomePage/
|      |- HomePage.react.js           |
|      |- HomePage.css                |      
|      |- HomePage.test.js            | <--- Each stateful component has the same files as the stateless ones
|      |- HomePageActions.js          |      and some actions, some constants and a reducer
|      |- HomePageConstants.js/       |       
|      |- HomePageReducer.js          |
|  |- reducers/
|    |- RootReducer.js    <--- Global Reducer
|- .htaccess
|- index.html
|- manifest.json
|- serviceworker.js

Still not sure what to rename the js/ folder to. Maybe react/ or redux/? Whats the best name, for both advanced developers who know whats going on and beginners who have no idea?

Member

mxstbr commented Dec 28, 2015

@zavelevsky that's very helpful, thanks! What do the constants.js in the routing folder do?

@hampusohlsson We have the app/ folder, which is like your src/ folder, and the build/ folder, which is like your dist/ folder. 👍
I've thought about doing the index.js thing, but it's non-obvious for beginners that it gets exported by default. We have a lot of magic happening already, so I'd rather leave it as it is for now.
I agree with the tests next to the components, that sounds good! It's dependent on #11 though, I have yet to find a nice way to unit test components... (@philihp pinged me on twitter about airbnb/enzyme which looks really nice. I hope he tries implementing it!)

This is what I'm currently looking at:

app/
|- assets/
|  |- img/
|  |- fonts/
|- css/    <--- Global CSS stuff, variables, mixins etc.
|  |- vendor/
|  |- utils/
|  |- main.css    <--- Main CSS file that imports everything
|- js/
|  |- actions/
|    |- AppActions.js    <--- Global Actions
|  |- constants/
|    |- AppConstants.js/    <--- Global Constants
|  |- components/    <--- Stateless components
|    |- NavBar/
|      |- NavBar.react.js     |
|      |- NavBar.css          | <--- Each stateless component has a .css file, a .js file and a .test.js file
|      |- NavBar.test.js      |
|    |- Button/
|      |- Button.react.js
|      |- Button.css
|      |- Button.test.js
|  |- containers/    <--- Stateful components
|    |- HomePage/
|      |- HomePage.react.js           |
|      |- HomePage.css                |      
|      |- HomePage.test.js            | <--- Each stateful component has the same files as the stateless ones
|      |- HomePageActions.js          |      and some actions, some constants and a reducer
|      |- HomePageConstants.js/       |       
|      |- HomePageReducer.js          |
|  |- reducers/
|    |- RootReducer.js    <--- Global Reducer
|- .htaccess
|- index.html
|- manifest.json
|- serviceworker.js

Still not sure what to rename the js/ folder to. Maybe react/ or redux/? Whats the best name, for both advanced developers who know whats going on and beginners who have no idea?

@hampusohlsson

This comment has been minimized.

Show comment
Hide comment
@hampusohlsson

hampusohlsson Dec 28, 2015

@mxstbr did not know about enzyme, looks good!

Regarding the renaming of the folder, I think it is hard to satisfy all developers, but I would not recommend using redux. There is another boilerplate erikras/react-redux-universal-hot-example, which uses redux for the ducks. As your project is gaining traction (congrats!) you have the opportunity to influence the recommended way of building redux apps. Especially for newcomers that might have seen the other repo, I think it would be a good idea to not mix it up too much as it will be confusing.

hampusohlsson commented Dec 28, 2015

@mxstbr did not know about enzyme, looks good!

Regarding the renaming of the folder, I think it is hard to satisfy all developers, but I would not recommend using redux. There is another boilerplate erikras/react-redux-universal-hot-example, which uses redux for the ducks. As your project is gaining traction (congrats!) you have the opportunity to influence the recommended way of building redux apps. Especially for newcomers that might have seen the other repo, I think it would be a good idea to not mix it up too much as it will be confusing.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 28, 2015

Member

A few thoughts later:

app/
|- assets/
|  |- img/
|  |- fonts/
|- global/    <--- Can also put global actions/reducers/... in here as well
|  |- css/
|     |- vendor/
|     |- utils/
|     |- main.css
|- components/
|  |- NavBar/
|     |- NavBar.react.js
|     |- NavBar.css
|     |- NavBar.test.js
|  |- Button/
|     |- Button.react.js
|     |- Button.css
|     |- Button.test.js
|- containers/
|  |- HomePage/
|     |- HomePage.react.js
|     |- HomePage.css
|     |- HomePage.test.js
|     |- HomePageActions.js
|     |- HomePageConstants.js    
|     |- HomePageReducer.js
|- .htaccess
|- index.html
|- manifest.json
|- serviceworker.js

Main changes:

  • No need for separate global actions/reducers/constants folders, lets put all of those plus the global CSS folder in a global/ folder.
  • No need for another wrapping folder around containers and components, those can live comfortably in the root folder

This one is much cleaner and smaller, while keeping the functionality fully intact.

The only question remaining for me is, where do we put app.js and rootReducer.js? In the global/ folder, hidden behind a wall?

Member

mxstbr commented Dec 28, 2015

A few thoughts later:

app/
|- assets/
|  |- img/
|  |- fonts/
|- global/    <--- Can also put global actions/reducers/... in here as well
|  |- css/
|     |- vendor/
|     |- utils/
|     |- main.css
|- components/
|  |- NavBar/
|     |- NavBar.react.js
|     |- NavBar.css
|     |- NavBar.test.js
|  |- Button/
|     |- Button.react.js
|     |- Button.css
|     |- Button.test.js
|- containers/
|  |- HomePage/
|     |- HomePage.react.js
|     |- HomePage.css
|     |- HomePage.test.js
|     |- HomePageActions.js
|     |- HomePageConstants.js    
|     |- HomePageReducer.js
|- .htaccess
|- index.html
|- manifest.json
|- serviceworker.js

Main changes:

  • No need for separate global actions/reducers/constants folders, lets put all of those plus the global CSS folder in a global/ folder.
  • No need for another wrapping folder around containers and components, those can live comfortably in the root folder

This one is much cleaner and smaller, while keeping the functionality fully intact.

The only question remaining for me is, where do we put app.js and rootReducer.js? In the global/ folder, hidden behind a wall?

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Dec 28, 2015

Member

I just noticed that pages can be both stateless or stateful. E.g. NotFoundPage and ReadmePage are stateless, while HomePage is stateful. Maybe we do need a seperate folder for pages? I'd like to keep them together if possible.

EDIT: Also, we should add a tests folder to the container folders, otherwise you end up with way too many files in there. An example stateful component, HomePage, would thus look like this:

HomePage/
|- HomePage.react.js
|- HomePage.css
|- HomePageActions.js
|- HomePageConstants.js
|- HomePageReducer.js
|- tests/
|  |- HomePageActions.test.js
|  |- HomePageReducer.test.js
Member

mxstbr commented Dec 28, 2015

I just noticed that pages can be both stateless or stateful. E.g. NotFoundPage and ReadmePage are stateless, while HomePage is stateful. Maybe we do need a seperate folder for pages? I'd like to keep them together if possible.

EDIT: Also, we should add a tests folder to the container folders, otherwise you end up with way too many files in there. An example stateful component, HomePage, would thus look like this:

HomePage/
|- HomePage.react.js
|- HomePage.css
|- HomePageActions.js
|- HomePageConstants.js
|- HomePageReducer.js
|- tests/
|  |- HomePageActions.test.js
|  |- HomePageReducer.test.js
@okonet

This comment has been minimized.

Show comment
Hide comment
@okonet

okonet Jan 19, 2016

Contributor

@mxstbr check #103 out

I think verbose file names are good because you'll be looking for stuff in IDE/editor by the name. But to reduce code for imports you could just use index.js file. In case of non-JS components (blocks in terms of BEM) you still could export a JS file with a reuire of CSS (which in turn requires images etc.).

Contributor

okonet commented Jan 19, 2016

@mxstbr check #103 out

I think verbose file names are good because you'll be looking for stuff in IDE/editor by the name. But to reduce code for imports you could just use index.js file. In case of non-JS components (blocks in terms of BEM) you still could export a JS file with a reuire of CSS (which in turn requires images etc.).

@CrocoDillon

This comment has been minimized.

Show comment
Hide comment
@CrocoDillon

CrocoDillon Jan 19, 2016

@okonet, I already mentioned the use of index.js as a proxy for nicer imports before, see #27 (comment)

It got overlooked though ;) Whatever the file names are I recommend this approach because you’ll have all exports for a module nicely listed in one small file (index.js).

CrocoDillon commented Jan 19, 2016

@okonet, I already mentioned the use of index.js as a proxy for nicer imports before, see #27 (comment)

It got overlooked though ;) Whatever the file names are I recommend this approach because you’ll have all exports for a module nicely listed in one small file (index.js).

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Jan 19, 2016

Member

Yeah, you were right it is so much nicer to name them index.js. Added credits to comment above, thanks for reminding me!

Member

mxstbr commented Jan 19, 2016

Yeah, you were right it is so much nicer to name them index.js. Added credits to comment above, thanks for reminding me!

@hampusohlsson

This comment has been minimized.

Show comment
Hide comment
@hampusohlsson

hampusohlsson Jan 19, 2016

@mxstbr glad you have realized the naming benefit of index.js :)

hampusohlsson commented Jan 19, 2016

@mxstbr glad you have realized the naming benefit of index.js :)

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Jan 21, 2016

Member

I put the actual components in the index.js instead of simply exporting them from there (one file less!), have been working with this for the past week on a commercial project and I'm loving it.

app
├── app.js
├── components
│   ├── Button
│   │   ├── Button.test.js
│   │   ├── index.js
│   │   └── styles.css
│   └── Img
│       ├── Img.test.js
│       └── index.js
├── containers
│   ├── App
│   │   ├── index.js
│   │   ├── logo.png
│   │   └── styles.css
│   ├── HomePage
│   │   ├── actions.js
│   │   ├── constants.js
│   │   ├── index.js
│   │   ├── reducer.js
│   │   ├── styles.css
│   │   └── tests
│   │       ├── actions.test.js
│   │       └── reducer.test.js
│   ├── NotFoundPage
│   │   └── index.js
│   └── ReadmePage
│       ├── index.js
│       └── styles.css
├── index.html
├── manifest.json
└── rootReducer.js

I still consider that in bad editors users won't be able to differentiate between the index.js, actions.js etc. files. Being held back by those few seems like a bad idea. If their editor can't do it, they can still rename them, but I recon enough people use Sublime/Atom/Brackets/... (which all show the path in the tab)


Does anybody have comments about the app structure, something they dislike? Otherwise I'll consider this issue resolved & will close it.

Member

mxstbr commented Jan 21, 2016

I put the actual components in the index.js instead of simply exporting them from there (one file less!), have been working with this for the past week on a commercial project and I'm loving it.

app
├── app.js
├── components
│   ├── Button
│   │   ├── Button.test.js
│   │   ├── index.js
│   │   └── styles.css
│   └── Img
│       ├── Img.test.js
│       └── index.js
├── containers
│   ├── App
│   │   ├── index.js
│   │   ├── logo.png
│   │   └── styles.css
│   ├── HomePage
│   │   ├── actions.js
│   │   ├── constants.js
│   │   ├── index.js
│   │   ├── reducer.js
│   │   ├── styles.css
│   │   └── tests
│   │       ├── actions.test.js
│   │       └── reducer.test.js
│   ├── NotFoundPage
│   │   └── index.js
│   └── ReadmePage
│       ├── index.js
│       └── styles.css
├── index.html
├── manifest.json
└── rootReducer.js

I still consider that in bad editors users won't be able to differentiate between the index.js, actions.js etc. files. Being held back by those few seems like a bad idea. If their editor can't do it, they can still rename them, but I recon enough people use Sublime/Atom/Brackets/... (which all show the path in the tab)


Does anybody have comments about the app structure, something they dislike? Otherwise I'll consider this issue resolved & will close it.

@okonet

This comment has been minimized.

Show comment
Hide comment
@okonet

okonet Jan 21, 2016

Contributor

@mxstbr it's not about showing path but about searching for a file. If you know you want to open Button component, you will be typing Button and not index, which will not include your index. So you will have to always type Button/index to open it. I think having one file less in repository isn't worth the pain you'll have every day. This is exactly what @nikgraf was talking about as well.

Contributor

okonet commented Jan 21, 2016

@mxstbr it's not about showing path but about searching for a file. If you know you want to open Button component, you will be typing Button and not index, which will not include your index. So you will have to always type Button/index to open it. I think having one file less in repository isn't worth the pain you'll have every day. This is exactly what @nikgraf was talking about as well.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Jan 21, 2016

Member

@okonet Ah you're right, I totally forgot about that, damn. Too many reasons for too many things to keep track of...

Thanks for reminding me, I'll have another think about it!

Member

mxstbr commented Jan 21, 2016

@okonet Ah you're right, I totally forgot about that, damn. Too many reasons for too many things to keep track of...

Thanks for reminding me, I'll have another think about it!

@ravinggenius

This comment has been minimized.

Show comment
Hide comment
@ravinggenius

ravinggenius Jan 21, 2016

@okonet Vim/Sublime/Atom each can find files via (case-insensitive) fuzzy searches. For instance you can type something like buttinx and be reasonably sure you will find what you're looking for, in this case app/components/Button/index.js. In many cases you can probably get away with typing less.

ravinggenius commented Jan 21, 2016

@okonet Vim/Sublime/Atom each can find files via (case-insensitive) fuzzy searches. For instance you can type something like buttinx and be reasonably sure you will find what you're looking for, in this case app/components/Button/index.js. In many cases you can probably get away with typing less.

@nikgraf

This comment has been minimized.

Show comment
Hide comment
@nikgraf

nikgraf Jan 21, 2016

Contributor

@ravinggenius just my personal experience: we always have a couple components with similar names: Trip, TripRow, Trips. So you don't always want to pick the first and it was just my personal perception, but it felt better once we switched to an index.js separated from the component. The index file can be super simple e.g. export default from './TripRow';

Contributor

nikgraf commented Jan 21, 2016

@ravinggenius just my personal experience: we always have a couple components with similar names: Trip, TripRow, Trips. So you don't always want to pick the first and it was just my personal perception, but it felt better once we switched to an index.js separated from the component. The index file can be super simple e.g. export default from './TripRow';

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Jan 21, 2016

Member

I agree with @nikgraf, especially if you're going with the "hardcore" containers there'll be lots of overlap between names.

I just don't think that adding a TripRow.react.js file will fix that, because then you have a TripRow.react.js, a Trip.react.js and a Trips.react.js. That's the same issue as with the index.js, which is that you have to write the full name of what you want, i.e. the question becomes typing Trip.r vs. typing Trip/i.

That doesn't really make a difference, but not having to add an "unnecessary" file each time you create a new component does make a difference, esp. for consistency with multiple devs, so I'm leaning towards the index.js-only-approach.

Member

mxstbr commented Jan 21, 2016

I agree with @nikgraf, especially if you're going with the "hardcore" containers there'll be lots of overlap between names.

I just don't think that adding a TripRow.react.js file will fix that, because then you have a TripRow.react.js, a Trip.react.js and a Trips.react.js. That's the same issue as with the index.js, which is that you have to write the full name of what you want, i.e. the question becomes typing Trip.r vs. typing Trip/i.

That doesn't really make a difference, but not having to add an "unnecessary" file each time you create a new component does make a difference, esp. for consistency with multiple devs, so I'm leaning towards the index.js-only-approach.

@mxstbr

This comment has been minimized.

Show comment
Hide comment
@mxstbr

mxstbr Jan 23, 2016

Member

I've decided to go with the index.js approach, for the reasons listed above. If you feel like that's a mistake, please let me know!

I've been working with the boilerplate (non-index.js version) the last week on a commercial product with lots of components and data handling, and I'm very happy with the current structure, so I'm closing this issue. Thanks for the great discussions and inputs everybody, I'm definitely going to link to this issue in the future!

Member

mxstbr commented Jan 23, 2016

I've decided to go with the index.js approach, for the reasons listed above. If you feel like that's a mistake, please let me know!

I've been working with the boilerplate (non-index.js version) the last week on a commercial product with lots of components and data handling, and I'm very happy with the current structure, so I'm closing this issue. Thanks for the great discussions and inputs everybody, I'm definitely going to link to this issue in the future!

@nareshbhatia

This comment has been minimized.

Show comment
Hide comment
@nareshbhatia

nareshbhatia Jun 12, 2016

Sorry for jumping in late, but as I am starting to understand react-boilerplate, I wanted to suggest an alternate approach to application structure that might be more intuitive.

This is what we currently have: all container components under containers and all presentational (or unconnected) components under components. This is widely known as "folder-by-type" structure. Unfortunately it doesn't scale well for even medium-sized applications - components for a single feature will be scattered all over the place. For example, a Dashboard container may have six presentational components sitting under the components folder intermixed with presentational components from other containers. This can quickly get out of hand.

Instead of breaking things by type, wouldn't it be better to break things by feature? Using this approach, the Dashboard component will be a top-level folder with the 6 presentational components under it as 6 child folders. If there is a strong need to distinguish container components from presentational components, the developer can follow some naming conventions similar to those suggested by Dan Abramov in his article, e.g. StoryContainer and Story. Also if there are components that are used in several places, they go under a shared folder. Based on these guidelines, the react-boilerplate example would be structured as follows:

app
 |--- App
 |--- HomePage
 |     `--- RepoListItem
 |--- FeaturePage
 `--- Shared
      |--- List
      |--- ListItem
      `--- ...

This seems much more intuitive to me because it keeps all related components together. What do you think?

nareshbhatia commented Jun 12, 2016

Sorry for jumping in late, but as I am starting to understand react-boilerplate, I wanted to suggest an alternate approach to application structure that might be more intuitive.

This is what we currently have: all container components under containers and all presentational (or unconnected) components under components. This is widely known as "folder-by-type" structure. Unfortunately it doesn't scale well for even medium-sized applications - components for a single feature will be scattered all over the place. For example, a Dashboard container may have six presentational components sitting under the components folder intermixed with presentational components from other containers. This can quickly get out of hand.

Instead of breaking things by type, wouldn't it be better to break things by feature? Using this approach, the Dashboard component will be a top-level folder with the 6 presentational components under it as 6 child folders. If there is a strong need to distinguish container components from presentational components, the developer can follow some naming conventions similar to those suggested by Dan Abramov in his article, e.g. StoryContainer and Story. Also if there are components that are used in several places, they go under a shared folder. Based on these guidelines, the react-boilerplate example would be structured as follows:

app
 |--- App
 |--- HomePage
 |     `--- RepoListItem
 |--- FeaturePage
 `--- Shared
      |--- List
      |--- ListItem
      `--- ...

This seems much more intuitive to me because it keeps all related components together. What do you think?

@oyeanuj

This comment has been minimized.

Show comment
Hide comment
@oyeanuj

oyeanuj Jun 12, 2016

FWIW, I have a structure similar to one mentioned by @nareshbhatia except for one change - instead of having a Shared folder, I have the shared components grouped by type of functionality provided by those components, i.e. Buttons, Forms, Cards etc. Currently using it on a medium-large scale application and has scaled well and made my life easier.

oyeanuj commented Jun 12, 2016

FWIW, I have a structure similar to one mentioned by @nareshbhatia except for one change - instead of having a Shared folder, I have the shared components grouped by type of functionality provided by those components, i.e. Buttons, Forms, Cards etc. Currently using it on a medium-large scale application and has scaled well and made my life easier.

@nareshbhatia

This comment has been minimized.

Show comment
Hide comment
@nareshbhatia

nareshbhatia Jun 12, 2016

Good to know, @oyeanuj. I have been googling for articles on application structure best practices. Here's two that I found to be very good:

  1. Rules For Structuring (Redux) Applications by Jack Hsu
  2. Angular2 Style Guide - This one is specific to Angular 2, but it focuses on the same basic issues. I like the structure of this guide because it not only lists the guidelines but also the reasons behind each.

nareshbhatia commented Jun 12, 2016

Good to know, @oyeanuj. I have been googling for articles on application structure best practices. Here's two that I found to be very good:

  1. Rules For Structuring (Redux) Applications by Jack Hsu
  2. Angular2 Style Guide - This one is specific to Angular 2, but it focuses on the same basic issues. I like the structure of this guide because it not only lists the guidelines but also the reasons behind each.
@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 30, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock bot commented May 30, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 30, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.