"common" directory and Angular web app strucutre

[Merott]

I just realised that with spell-webapp, unlike the original ngbp, there is no common directory under src. Likewise, for the warlock branch of ngbp, you've moved plusOne out of common and into the app directory, alongside the rest of the app files.

I understand spell-webapp isn't specific to AngularJS, so perhaps warlock-spell-angular could add common back to the globs, but again, having reusable components isn't specific to AngularJS either.

I'm wondering what was your reason for this change, and what directory structure you are proposing when it comes to reusable components such as plusOne?

[joshdmiller]

Great question! It actually was intentional, though I documented it nowhere. There was a long discussion I started on yeoman/generator-angular#109 and as a result of that and of the contributions of many others, Google came out with a set of recommendations for structuring angular apps.

These recommendations largely followed my conception but instead moved the "common" components inline with the app files. I think they're right; if all components are drag and drop, it won't make a difference and the distinction was confusing to a lot if people.

There's actually one additional change in the structure from ngbp: the src/less/main.less file is now src/app/app.less. If components are drag-and-droppable, they should have their stylesheets with them too. The less should also be gathered automatically instead of requiring manual inclusion, which is something a few people requested in ngbp, but it's not implemented yet.

What are your thoughts?

[Merott]

I definitely preferred having the common directory for my directives, services, etc, and I could be misinterpreting the Google Doc you referenced, but it seems to me that that the proposed directory structure still has a directory for common code, albeit under the name components instead.

We group elements of an application either under a "components" directory (for common elements reused elsewhere in the app), or under meaningfully-named directories for recursively-nested sub-sections that represent structural "view" elements or routes within the app:

A components directory contains directives, services, filters, and related files.

Now, they've put components directly next to app-specific files, such as app.js, but it's definitely suggesting having one common parent directory for all directives, services and other reusable components.

I would personally keep the code that provides the functionality of that specific app in its own directory. This directory would almost mirror the navigational structure of the app, and its modules are unlikely to be used in another app, given that they are the app. That would be the app directory of the original ngbp.

I think the question here really is where you put that common or components directory, whether it goes inside the app folder, or next to it, like in the original ngbp.

I personally prefer the latter (and common instead of components) - basically the original directory structure for ngbp. This could easily be made into a configurable path with Warlock.

I noticed the app.less change too, and I do like that!

[joshdmiller]

First, let's ensure we're on the same page functionally. Regardless of where the components go - whether in src/app, in src/app/common, or in src/common - Warlock and all its spells will treat it identically to everything else in the app, including the routes and whatnot. There's nothing functionally or physically different about component code, so the difference we're discussing is philosophical.

Second, yes - the recommendation puts the components nested in src/app/components. I was mistaken.

Alright, so let's assume for a moment that we go with a parent directory for reusable components. I think having it outside src/app, as I originally did in ngbp, is confusing; I was asked many times by the community where to put various stuff. I think the problem here is that truly "reusable" components usually arrive at the application in a controlled way (e.g. through Bower) and are independently developed and tested. Using the src/components hierarchy may lead some to think they're developing strong, reusable components when they're not - the components, after all, are developed and tested alongside the application, usually with an unavoidably narrow focus. Without developing and testing toward independent specs, the components really are part of the app - even if we want to keep them a little separate.

All that said, I don't object to placing plusOne, etc., inside src/app/common (which, I agree, is a more descriptive name than "components"). While placing them outside src/app is trivial with Warlock, I think it's a little confusing and misleading. What are your thoughts here?

As for LESS, yes - I think moving it was a great suggestion. The problem I have so far with the auto-importing is that the only reasonable way to do it is to concatenate before compiling. But that will make it difficult to find syntax problems, so that's no good. That's the same situation as present, however. I had the thought of being able to specify two globs for LESS - one for common stuff like variables and mixins, and another for the actual styles - that would get concatenated before compilation on an individual basis. The advantage is that styles in build/assets/styles/about/about.css will be the styles from src/app/about/about.css, so it's all easy to locate - particularly with source maps. But that feels like it may be kludgey - I don't know.

[Merott]

I think if we're to put the common directory alongside the rest of the app files, shouldn't assets and index.html be the same? The recommendation also puts index.html alongside components and the rest.

So, if we go by that and move assets and index.html into app too, that makes src redundant because it wouldn't contain anything other than the app directory.

- src/
    |- app/
        |- assets/
        |- common/
        |- home/
        |- about/
        |- index.html
        |- app.js
        |- app.less
        |- app.spec.js
- node_modules
- package.json
- warlock.json    

Given that src is redundant, we could move content of app into src and remove app, or move app up a level and remove src

- app/   (or src/ depending on your preference)
   |- assets/
   |- common/
   |- home/
   |- about/
   |- index.html
   |- app.js
   |- app.less
   |- app.spec.js
- node_modules
- package.json
- warlock.json    

To me, that seems to be the directory structure recommended.

Although I don't necessarily dislike that directory structure, what I find unpleasant is that my home and about directories (what they've called subsection directories) live alongside common and assets, which are different types of directories (not subsection).

So, I'd probably break them up inside the app folder, but as long as Warlock looks in the app directory for files to process, it shouldn't matter how I organise files and folders in app.

[joshdmiller]

Ah, reductio ad absurdum. Nice.

But this is why I made sure we were talking about philosophical differences instead of functional differences when comparing app and common. The same doesn't apply to assets and index.html; we process them completely differently, so it definitely makes the most sense to keep them separate. More directly, our build process doesn't give a crap about the difference between src/app and src/common, but it does about the difference between those and src/index.html and src/assets.

All that said, I don't care all that much about where the common stuff goes. My only real gripe is that putting it outside src/app doubles the amount of globbing patterns in nearly every spell; spell-webapp has to look in both for JavaScript and for CSS; spell-coffeescript for CS; spell-less and spell-sass each for their style files; spell-angularjs for the templates; and any spell that anyone creates that builds on web apps will in the future have to include both globbing patterns just to ensure everything's caught. And should the user choose a slightly different structure, there is now one more path he has to change. This seems, to me at least, to be a high price to pay to have one directory of identically-processed files put outside src/app for purely ideological reasons.

But perhaps there is a compromise. Right now, all the src/app-based globbing patterns use a concatenation of the paths.source_app variable with a globbing pattern. If instead we offered an array of paths.source_app that all globbing patterns would inherit, then we've removed most of the extra work. This would require a change to the engine, so it would definitely need to be something the community felt strongly about - otherwise they can change the individual globs themselves in their warlock.json:

{
  "paths": {
    "source_cmn": "<%= source %>/common"
  },
  "globs": {
    "source": {
      "js": [
        "<%= paths.source_cmn %>/**/*.js",
        "!<%= paths.source_cmn %>/**/*.spec.js"
      ],
      "css": [ "<%= paths.source_cmn %>/**/*.css" ],
      "less": [ "<%= paths.source_cmn %>/**/*.less" ],
      "templates": [ "<%= paths.source_cmn %>/**/*.tpl.html" ],
      "html": [
        "<%= paths.source_cmn %>/**/*.html",
        "!<%= paths.source_cmn %>/**/*.tpl.html"
      ],
      "coffeescript": [
          "<%= paths.source_cmn %>/**/*.coffee",
          "<%= paths.source_cmn %>/**/*.coffee.md",
          "<%= paths.source_cmn %>/**/*.coffee.markdown",
          "!<%= paths.source_cmn %>/**/*.spec.coffee",
          "!<%= paths.source_cmn %>/**/*.spec.coffee.md",
          "!<%= paths.source_cmn %>/**/*.spec.coffee.markdown"
        ]
    }
  }
}

[Merott]

You're right, this is really about preferences around code organisation.

"our build process doesn't give a crap about the difference between src/app and src/common, but it does about _the difference between those and src/index.html and src/assets_" (emphasis added)

Could you be a little more specific about that part? For example, what would happen if I added this to my warlock.json:

"paths": {
   "source_app": "<%= paths.source %>"
}

In effect, that tells Warlock to treat src as the place where all the app files, and make no distinction between src/ and src/app/, src/app/common, or src/common. I've tested it with a minimal app, and it worked fine, but perhaps there is something that you foresee going wrong with that in a more complex app.

Thanks.

[joshdmiller]

My point wasn't about something going wrong. Out of the box with a minimal app, nothing should go wrong. That said, if there is a file matching an app glob in src/assets, it will be picked up by the build, which was a problem for some in the past (e.g. ngbp/ngbp#106).

My point, however was simply that it is imprecise. We've defined a glob to do one set of tasks for everything in the assets directory, another for src/index.html, and a third for specific globs inside src/app. Putting all three in the same directory would unintentionally cross the streams. This may or may not be a problem for everyone, but it would definitely be unexpected and I don't think we can predict what may happen in the wild. I'm definitely in favor if having at least assets in its own directory; index.html shouldn't be a problem.

Now if I could ask a probing question: what's the elevator pitch for having common outside your standard app files? I don't have an opinion, so I'm basing everything I've said on pragmatism within the build system. However, I'd like to understand your perspective.

[Merott]

I don't have a problem with common being alongside the app files, I would just re-organise app a little, so if I wanted to stick with Warlock's expectations, this is what I'd do:

- src/
   |- app/
      |- areas/
         |- home/
         |- about/
      |- common/
         |- directives/
         |- services/
      |- app.js
      |- app.less
      |- app.spec.js
   |- assets/
   |- index.html
- node_modules/
- package.json
- warlock.json   

That is purely for organisation, and has no functional purpose. It's because common isn't the same sort of thing as home and about, so I wouldn't put them three together in one place.

I don't mind that, but the structure may feel overly nested. When common was outside, I didn't feel the need for areas (can't think of a better name for it - potentially a spinoff question!).

To be honest, common inside the app directory makes sense, and I can see why it being outside would cause confusion for people new to ngbp - I guess I got accustomed to it after a while.

So, I was trying to understand the justification for common being inside app, and I think you've already answered that.

[fubupc]

@Merott @joshdmiller

Your discussion is really great! I had the same confusion as Merott about "components/common the same folder as home/about". And I have an another similar confusion:

e.g. I have an app with a header/main/footer layout and 2 pages home/about. And header/footer is common between 2 pages. Then there maybe some structure like the following:

|- app/
      |- header/    /* a subsection of app, same level with home/about */
      |- footer/      /* a subsection of app, same level with home/about */
      |- home/
      |- about/
      |- common/
         |- directives/
         |- services/
      |- app.js
      |- app.less
      |- app.spec.js
   |- assets/
   |- index.html

or:

|- app/
      |- header/    /* a subsection of app */
      |- footer/      /* a subsection of app */
      |- main/       /*  a subsection of app, same level with header/footer */
         |- home/   /* Home is subsection of main? */
         |- about/
      |- common/
         |- directives/
         |- services/
      |- app.js
      |- app.less
      |- app.spec.js
   |- assets/
   |- index.html

or:

|- app/
      |- home/
      |- about/
      |- common/
         |- header/    /* a "reusable component" across the app, so resides in common/components */
         |- footer/      /* a "reusable component" across the app, so resides in common/components */
         |- directives/   /* but it feels strange that "directives/services" the same level with "header/footer" */
         |- services/
      |- app.js
      |- app.less
      |- app.spec.js
   |- assets/
   |- index.html

So what do you think the best structure among the above?

Thanks!