icholy / ember-brunch

80 stars 23 forks source link

Cleanup the require() calling in controllers, templates, views and such. #6

Closed flexd closed 11 years ago

flexd commented 11 years ago

This is untested/currently does not work (it does not find require-dir), but I think this is a good approach to doing things.

At the very least we should be using index.js files in the folders themselves instead of having a bunch of js files with the same name as the folders. require('controllers') will require controllers.js or the file in folder controller/index.js :-)

Anyway, using something like I suggest here we would have to re-type things a whole lot more.

flexd commented 11 years ago

Instead of using require-dir, we could use something like this:

require('fs').readdirSync(__dirname).forEach(function (name) {
  if (name !== 'index.js' && name.substr(name.length-3) === ".js") {
    var name = name.substr(0, name.length - 3);
    Object.defineProperty(exports, name, {get: function () {
      return require("./" + name);
    }, enumerable: true});
  }
});

Taken from https://github.com/creationix/creationix/blob/master/index.js , which is MIT licensed. That way there are no external requirements.

icholy commented 11 years ago

I agree with using index.js. I don't want to have it load everything in the directory though. There are often times when there are files I don't want to include.

icholy commented 11 years ago

There's the tiny issue of what to rename the index route => app/routes/index.js

flexd commented 11 years ago

If we use what I posted in my last comment we could just say that anything with a _ in the beginning of the filename is not loaded? Or something like that. And yeah, I thought about the index route problem too, not sure what is the answer to that yet.

icholy commented 11 years ago

I was also considering the _ prefix to ignore files. It would mesh well with the way brunch works.

flexd commented 11 years ago

Instead of having a index.js in each folder, we could just write something (like the above) that requires everything in controllers, models, views and templates, with the exception of the files starting with _.

Instead of doing require('controllers') and so on in another file.

icholy commented 11 years ago

the problem is that we'll be limited to flat directories. In more complicated applications, I have nested views and that nesting is reflected in the directory structure.

views/
  subview1/
      index.js // Subview1IndexView
      controls.js // Subview1ControlsView
  subview2/
      index.js // Subview2IndexView
      bar.js // Subview2BarView
icholy commented 11 years ago

I need to give this some thought. Closing for now.

icholy commented 11 years ago

what about something like this?

var files = {
  templates:   [ 'foo', '_some_partial' ],
  models:      ['foo', 'bar', ], 
  controllers: ['foo'],
  views:       ['foo'],
  routes:      ['users']
};

Object.keys(files).map(function (key) {
  return files[key].map(function (file) {
    return key + '/' + file;
  });
}).reduce(function (acc, arr) {
  return acc.concat(arr);
}, []).forEach(require);
flexd commented 11 years ago

Hm, I'm not sure. Then you still have to define which controllers, views and such you want to be included, which is what I wanted to eliminate in the first place.

I would rather see something inverted, so files starting with _, or mentioned in that files array aren't included?

icholy commented 11 years ago

You can disable brunch modules in config.coffee

modules:
    wrapper: false
    definition: false

Then you would have to go through and remove all the var App = require('app')

This PR will let the handlebars pre-compilation work correctly without modules. https://github.com/icholy/ember-handlebars-brunch/pull/6