Settings constants for all property names, worthwhile or over-engineering?

As our code-base is growing, so too is the assets size and load times. One area of optimisation I have considered is property names. I imagine that the compiler is unable to minify string literals, not to mention the duplication aspect.

Example of status quo:

// controller
name:  computed("firstName", "secondName", function () {  
   return this.get("firstName") + " " + this.get("secondName");
}),

firstName: reads("model.firstName"),

secondName: reads("model.secondName"),

getName: function () {
  return this.get("name");
}

With constants:

 // controller
    import { MODEL_PROPS } from "..path to model";
    const PROPS = {  
       NAME: "name",
       MODEL: "model"
    }
...
[PROPS.NAME]:  computed(MODEL_PROPS.FIRST_NAME, MODEL_PROPS.SECOND_NAME, function () {  
   return this.get(MODEL_PROPS.FIRST_NAME) + " " + this.get(MODEL_PROPS.SECOND_NAME);
}),

[MODEL_PROPS.FIRST_NAME]: reads(`${PROPS.MODEL}.${MODEL_PROPS.FIRST_NAME}`),

[MODEL_PROPS.SECOND_NAME]: reads(`${PROPS.MODEL}.${MODEL_PROPS.SECOND_NAME}`),

getName: function () {
  return this.get(PROPS.NAME);
}

This would be a fairly significant undertaking and one that we’d work towards in stages. However, I would like to know if anyone here has tried to engineering their app to this degree and what your experience has been. Is it worthwhile, or even achievable without grinding development to a halt?

that is over-engineering. over-enginnering over simplicity is self defeating on the long run. Also as you rightly mention compiler is unable to minify string literals, not to mention the duplication aspect

There is a big trade pff between cleverness and readability. What you suggest would make the code unreadabble. However, there are some cases when using contants like that actually help.

First with your status-quo example you can leverage the bracket syntax:

name: computed(‘model.{firstName,lastName}’, ...)

Which is much easier to read. Second, in some cases when you have to a known list that would be easier to read as an array at the top of the file instead of inline in the computed construction then this would help:

const LONG_LIST_OF_PROPS = Ember.String.w(`
  foo
  bar
  baz
`.trim());

foobar: computed(...LONG_LIST_OF_PROPS, ...)
1 Like

The way to try to optimize this kind of thing is not doing it by hand and massively inconveniencing every person who ever needs to read your code again. It’s applying a compiler that can do the optimization you want.

The default minimization every Ember app gets in production builds is provided by Uglify. Uglify has a lot of options that can be tweaked if you want it to be more aggressive, at the cost of doing things that may be safe in your app (if you’re careful), but not safe in every possible app (so we can’t turn them on by default for every app).

You can also drop uglify entirely and add something different. Like the Google Closure Compiler, which can do very aggressive optimization if you accept some extra rules about how you write your code.

Another option is to author in Typescript (which can be adopted incrementally, since valid Javascript is already valid Typescript) and use TsMinifier. The benefit here is that in Typescript you can prove which identifiers can’t be used by outside code (because they’re marked private), which lets a Typescript minifier do the kind of optimizations you’re suggestion.

2 Likes

One thing to take into consideration is that const are not truly constants. They’re still mutable. So you maybe giving false idea with the ALL_CAPS naming.

Also, to give you another perspective, AirBnb eslint (which is the most popular styleguide out there) prefers to use const over let.