linnovate / mean

The MEAN stack uses Mongo, Express, Angular(6) and Node for simple and scalable fullstack js applications
http://mean.io
12.12k stars 3.45k forks source link

Too many files with identical names #373

Closed adrianedworthy closed 9 years ago

adrianedworthy commented 10 years ago

When switching to another file I find it common to have to cycle through multiple files of the same name to find what I want which can get tiresome and frustrating after a while.

too-many-identical-filenames

Can we add some distinction to the filenames? Like:

articles.init.js articles.controller.js articles.routes.js articles.resource.js

It would also be nice to tell the difference between public and server controllers and routes. Maybe capitalise server types?

articles.CONTROLLER.js

Or maybe there's a better way?

jackhsu978 commented 10 years ago

+1

dudo commented 10 years ago

+1

fyockm commented 10 years ago

For more reference: http://hastebin.com/kategokiju.vbs (via @adrianedworthy) Looking for other suggestions on how to improve.

adrianedworthy commented 10 years ago

Further explanation of that hastebin link... Presuming most packages are likely to be concise it seems a little overkill to have so many folders. I propose that the files are not buried in subdirs by default, but optionally you may use subdirs to group similar files - allowing complex dir structures for more complex packages.

jackhsu978 commented 10 years ago

Since AngularJS is a big part of MEAN, I think it would make sense to follow what AngularJS team recommends: An AngularJS Style Guide and Best Practice for App Structure

adrianedworthy commented 10 years ago

It would appear that the hastebin link is now broken, so here is the previously proposed structure again:

Suggestion for Default Package Paths

articles/
  public/
    assets/
      css/
    views/
      index.html
    articles.config.js
    articles.controller.js
    articles.model.js
    articles.routes.js
  server/
    articles.CONFIG.js
    articles.CONTROLLER.js
    articles.MODEL.js
    articles.ROUTES.js

But We Want More Than One Controller/Model/Whatever

If we need multiple files of the same type simply add a "controllers" dir where necessary and everything in it is loaded when the controllers are.

articles/
  public/
    assets/
      css/
    controllers/
      articles.controller.js
      whatever.controller.js
    views/
      index.html
    articles.config.js
    articles.model.js
    articles.routes.js
  server/
    articles.CONFIG.js
    articles.CONTROLLER.js
    articles.MODEL.js
    articles.ROUTES.js
adrianedworthy commented 10 years ago

@jackhsu978 what parts of the Angular Team's recommendations do you think are relevant here? Could you provide an example?

fyockm commented 10 years ago

Looks to me like they propose a much more modular approach. That is, instead of all the controllers together, put all the articles together, all the things together, etc. We're doing this in a way already with the packages, but it gets mucked inside the package.

The other main recommendation I noticed was to use article-controller.js instead of article.controller.js. We could still keep the uppercase for server-side, as article-CONTROLLER.js, but not sure I like that as much.

hbzhang commented 10 years ago

I would say following Ruby on Rails convention that within controller it should be such as articles_controller.

On Wed, Apr 16, 2014 at 9:36 AM, Drew Fyock notifications@github.comwrote:

Looks to me like they propose a much more modular approach. That is, instead of all the controllers together, put all the articles together, all the things together, etc. We're doing this in a way already with the packages, but it gets mucked inside the package.

The other main recommendation I noticed was to use article-controller.jsinstead of article.controller.js. We could still keep the uppercase for server-side, as article-CONTROLLER.js, but not sure I like that as much.

— Reply to this email directly or view it on GitHubhttps://github.com/linnovate/mean/issues/373#issuecomment-40598957 .

adrianedworthy commented 10 years ago

@fyockm, you say it gets mucked inside the package? Do you mean because of the public and server folders? I think it's important to keep a clear division between Angular and Express.

One alternative to using uppercase to indicate server side files is to do article-p-controller.js and article-s-controller.js. However, uppercase is very distinguished from lowercase and is easier to filter into your brain when you are looking at a bunch of open server and client side files.

Couple of questions:

What about this:

articles/
  client/
    css/
    img/
    views/
      index.html
    articles-config.js
    articles-controller.js
    articles-model.js
    articles-routes.js
  server/
    articles-CONFIG.js
    articles-CONTROLLER.js
    articles-MODEL.js
    articles-ROUTES.js
jackhsu978 commented 10 years ago

For the client side, I think we should follow what AngularJS recommends because it's probably going to be the convention of AngularJS apps:

For the server side, I like RoR's approach like articles-controller.js. To make the name unique, maybe add .server in the end like articles-controller.server.js, but I'm not sure about this.

I also like having bower_components and site-wide static assets be separated from AngularJS application

assets/
  img/
bower_components/
  angular/
client/
  components/
    articles/
      articles-service.js
  articles/
    articles-create.html
    articles-edit.html
    articles-list.html
    articles-view.html
    articles.css
    articles-controller.js
    articles.js
  app-controller.js
  app.css
  app.js
  header.html
  index.html
server/
  controllers/
    articles-controller.js
  models/
    article.js
  routes/
    articles-route.js
  views/
    index.html
dudo commented 10 years ago

I like the idea of keeping it as close as we can to how angular suggests. If the team at google helped come up with a certain convention, I'm inclined to say that it's pretty solid. How you propose, @adrianedworthy , isn't really how google has their's laid out, though. This is how I really want to use the packages, btw, but I realized that's not their intention.

This is google's layout:

sampleapp/
    app.css
    app.js                                    # top-level configuration, route def’ns for the app
    app-controller.js
    app-controller_test.js
    index.html
    components/
        adminlogin/                                
            adminlogin.css                    # styles only used by this component
            adminlogin.js                     # optional file for module definition
            adminlogin-directive.js                         
            adminlogin-directive_test.js        
        private-export-filter/
            private-export-filter.js
            private-export-filter_test.js
        userlogin/
            somefilter.js
            somefilter_test.js
            userlogin.js
            userlogin.css                
            userlogin.html                
            userlogin-directive.js
            userlogin-directive_test.js
            userlogin-service.js
            userlogin-service_test.js                
    subsection1/
        subsection1.js
        subsection1-controller.js
        subsection1-controller_test.js
        subsection1_test.js                         
        subsection1-1/                        
            subsection1-1.css
            subsection1-1.html
            subsection1-1.js
            subsection1-1-controller.js
            subsection1-1-controller_test.js
        subsection1-2/                        
    subsection2/
        subsection2.css
        subsection2.html
        subsection2.js
        subsection2-controller.js
        subsection2-controller_test.js
    subsection3/
        subsection3-1/
    etc...

You want people to adopt this stack as quickly and easily as possible. Easy is quick, as quick is easy... 'Everyone' seems to be going in the Mongo/Node/Angular direction (and node typically includes express) so we're all sort of headed in the same direction. The easier we make it for people to jump on board with MEAN.io, specifically, the better for you guys. I'm all about people rallying around a single node, so to speak, because it makes the community that much stronger. I digress... back to folder structure.

As it relates to MEAN, and this thread:

mean/
    app/                                     # group the application specific scope into the app dir... this is kind of the 'component' section of google's example
        client/
            stylesheets/
                app.css
                user.css
                article.css
            img/
            views/                           # global views stay public, but at the app level
                includes/
                    footer.html              # not foot and head... I think those should be included directly in default.html, for better visibility
                    header.html              # header and footer being the actual visual elements that show up on every page, typically
                layouts/
                    default.html   
                404.html                     # etc 
                about.html
                contact.html                 # static pages, etc (I could also see these as their own components) 
                index.html               
            app-routes.js
            app-controller.js                # angular is big with the '-' so use dash notation for things in public
            app.js
        server/                              # now we keep config and model at the server level (config only at the app level)
            config/
                env/                         # etc
                system/
                bower.json
                gruntfile.js
                server.js
                routes.js                    # app is the only place we should need server routes for express. I like the idea of keeping this in a single place, and I like it under config (again, coming from rails). this could easily be a dir of individual express routes as well.
            app-rest.js                      # RESTful controller routes (original thought was app-controller.js, see below *). no model at the app level. this would be equivalent to a pages_controller in rails, to handle static pages.
        test/
            karma/                           # app level unit tests, but keep tests in their appropriate module
    user/                                    # start adding your subsections. user, specifically, could be app level, but since it is an object you can perform crud operations on, i think it should be modulated (also, not every site will need log ins)
        client/
            views/ 
                index.html                   # for the list view of users (could be called list, too)
                show.html                    # could be called detail, but I'm coming from rails
                _part.html                   # partials included here as well
            user-controller.js
            user-routes.js
            user.js
        server/
            user-rest.js                     # config and routes are kept to the app level
            user-model.js
        test/
            mocha/                           # component level tests for the user
    article/                                 # to add the article example that mean uses
        client/
        server/
        test/
    etc/
        client/
        server/
        test/

I'm ok with client or public... they're the same to me.

I could see pulling assets out of the subsections, and only have images / css at the app level. (edited to reflect this)

This is my 30,000ft take on how the folder structure should work, but the idea is to keep it very modular. Only the items in the views might end up with similar names, but that's pretty contextual. With this kind of layout, you'll typically only have 1 main folder open while working on a section, with a quick glimpse for server and client interactions.

adrianedworthy commented 10 years ago

Revised Proposal

/node_modules/
    meanio/ ← Main meanio stuff
        ...
    meanio-404/ ← This is a core package
        ...
    meanio-auth/ ← This is a core package
        client/
            css/
                ...
            img/
                ...
            views/
                ...
            index.html
            auth-controller.js
            auth-defaults.js
            auth-service.js
            auth-routes.js
        server/
            auth-CONTROLLER.js
            auth-DEFAULTS.js
            auth-MODEL.js
            auth-ROUTES.js
        test/
            ...
    meanio-system/ ← This is a core package
        client/
            css/
                default.css
            index.html
            system-controller.js
            system-defaults.js
            system-routes.js
        server/
            layouts/
                default.html
            system-CONTROLLER.js
            system-DEFAULTS.js
            system-ROUTES.js
        test/
            ...
bower_components/
    ...
packages/
    auth/ ← This is a local package
        server/
            auth-CONFIG.js ← Override vars within auth-DEFAULTS.js (in meanio-auth)
    system/ ← This is a local package.
        server/
            layouts/
                default.html ← Override layout file
Gruntfile.js
karma.conf.js
package.json
README.md
index.js ← This will contain a couple of lines launching the meanio framework

So the intention here is to allow core and contrib package files to be overridden. Config vars can be overridden also. You can even just uninstall core packages and replace them with contrib, or with local packages. What do you guys think?

Points for Debate

@Mallanaga interesting idea to give the server files different names, I think they'd have to be really concise to stray from the typical MVC names though.

@jackhsu978 doing auth-controller-server.js is another good suggestion.

I'd argue that capitalising them would be incredibly visually distinct, making it obvious at a glance which files you have open in your IDE.

I'm not convinced about having global client/server folders and I'm advocating that we get rid of them. There is little reason why every part of a website can't be modular. Of course those global folders should still work if present so that meanio is backwards compatible.

dudo commented 10 years ago

can you include articles in your example, so I can get more of a feel for the direction you're headed? I'm digging it, quite a bit...

adrianedworthy commented 10 years ago

@Mallanaga the articles structure would be the same as the meanio-auth package. If it's a core or contrib package, it would go in node_modules/ prefixed with "meanio-". If it's a local package it would go in packages/, which is not gitignored.

Glad to hear you approve, don't hesitate to criticise though - the more we refine this the better. We should try to avoid further path changes later if we can help it.

Here's another thing:

Exploding Files

Packages are very likely to be very lean, so polluting them with subfolders containing only 1 file each is obtrusive for the majority of packages. To keep them lean without sacrificing the potential for more complex packages, we could have “exploding files”.

Let’s say you only want one controller for articles, you would use:

articles-controller.js

Later, we realise “Oh no! We need 4 controllers for articles! What now?”. We simply use the exploding file strategy, create a folder called controllers/ and place multiple controller files inside.

controllers/
    articles-controller.js
    comments-controller.js
    foo-controller.js
    bar-controller.js

So "exploding file" refers to a file in our structure that can become a subdir containing multiple files of the same type and still remain valid/best practice.

dudo commented 10 years ago

Well... articles isn't a core package. That's why I'm curious how you're thinking it'd be laid out. articles are an example of how to handle CRUD, and to give you a starting point of sorts for creating your own models / views / controllers. This is why I'd like to see them top level, and not tucked into the packages or the node_modules.

I like the idea of exploding files. That makes perfect sense.

fyockm commented 10 years ago

@Mallanaga You're right. articles should be an example, not core. I had a conversation with @ellman about that very thing yesterday. We plan a number of examples (or patterns), to be installed into the packages/ folder via the CLI. That way, developers can take them and easily modify. If articles were a "Contrib" package, it would be installed under node_modules where it shouldn't be modified.

@adrianedworthy has already done some work on the exploding file concept.

mattdeluco commented 10 years ago

I could probably live with sets of two identically named files - one server side, the other client side, since I tend to work on only one or the other at a time. "ArticlesController.js" for example, on both client and server sides.

I'm not sure what that file name convention is called, but given that the Angular controller name is "ArticlesController" I'd much prefer "ArticlesController.js" over "articles-controller.js" or any filename for that matter.

In my own project, one convention I use server-side is to use API in my routes filenames, for example "ArticlesAPI.js" rather than "ArticlesRoutes.js", client-side could name the file "ArticlesStates.js"

The following scheme only ends up with two identical file names (the controllers):

articles/
  app.js
  bower.json
  package.json
  README.md
  client/
    ArticlesController.js
    ArticlesDirectives.js
    ArticlesServices.js
    ArticlesStates.js
    styles/
      ...
    views/
      ...
    tests/
      ...
  server/
    ArticlesAPI.js
    ArticlesController.js
    ArticlesModel.js
nicolaspanel commented 10 years ago

@fyockm : it seems that your last commit introduced an error (see comments here )

mattdeluco commented 10 years ago

@nicolaspanel See https://github.com/linnovate/mean-cli/commit/1c969f7 - updating the npm packages of your mean project should fix any related issue you're running into.