tower-archive / tower

UNMAINTAINED - Small components for building apps, manipulating data, and automating a distributed infrastructure.
http://tower.github.io
MIT License
1.79k stars 120 forks source link

should minify assets when bundling for production env #346

Closed svasva closed 10 years ago

svasva commented 11 years ago

This is how I monkeypatch my Cakefile for now:

# This bundles all of your assets into neat little files
task 'assets:bundle', ->
  invoke 'environment'
  Tower.ApplicationAssets.bundle(minify: (process.env.NODE_ENV is 'production'))

But that doesn't work as expected, having some trouble with minified variables:

Uncaught Error: Constant 'App.r' wasn't found
Uncaught Error: Constant 'App.n' wasn't found
lancejpollard commented 11 years ago

This is a bug that has to do with the coffee-script extensions in ./coffee-inheritance.js. We just need to change this (coffee-script output for an App.Address model):

App.Address = (function(_super) {
  // since this is being declared as a local variable, uglify.js will minify it,
  // making it App.r or some random shortened name, vs. App.Address.
  var Address;

  function Address() {
    return Address.__super__.constructor.apply(this, arguments);
  }

  Address = __extends(Address, _super);

  // ...

  return Address;

})(Tower.Model);

If you would like to fix this let me know, I'll do what I can to help. Won't be able to get to this for a bit (workaround at the bottom of this post).

One approach is to change this line:

Address = __extends(Address, _super);

to something like this:

Address = __extends('Address', _super);

That way it passes the string 'Address' to __extend, and we can use that to set the class name. The way we're hooking into this is found here:

packages/tower-support/shared/class.coffee#L8-12

__extend: (child) ->
  object = Ember.Object.extend.apply @
  object.__name__ = child.name
  @extended.call(object) if @extended
  object

By passing a string instead of the result of var Address into that function, the __extend method above would just do this:

__extend: (name) ->
  object = Ember.Object.extend.apply @
  object.__name__ = name
  @extended.call(object) if @extended
  object

To make this change, we have to modify the ./coffee-inheritance.js file so it passes a string, like above:

Address = __extends('Address', _super);

I haven't dug into the coffeescript compiler that hardcore so haven't been able to implement this. Would love help.

Note: A temporary workaround is to set the @__name__ property on your model, like this:

class App.Address extends Tower.Model
  @__name__: 'Address'
edubkendo commented 11 years ago

@viatropos arent we going to be moving away from the custom coffeescript compile stuff? should anyone be worrying about fixing that stuff or would it be better just to use temp. workarounds until we change the api?

lancejpollard commented 11 years ago

@edubkendo Ideally I'd think it would be useful to solve the custom coffee-script stuff (maybe using CoffeeScriptRedux like you mentioned). If we can get it to work just as we originally desired (with custom overrides for extends for Ember, etc.) then if anyone who really loved CS and it's class declaration syntax used tower, they would be greeted with a super complete API.

I would like to support those things, but it would be purely just to round out the experience with the API for CS users. This kind of stuff will really just be extras to the API; everything will still default to JS/CS like you're saying.

So I personally probably wont work on this "CS API stuff" in the short term, but it's cool if anyone wants to make a pull request for this. I will still be working on the JS API (which will make updates to the CS api too).