outmoded / hapi-contrib

Discussion forum for project contributors
Other
78 stars 25 forks source link

Style guide: getters and hapi-scope-start #108

Closed devinivy closed 5 years ago

devinivy commented 7 years ago

I'd like to propose that we allow one-line getters. The impetus for the line at the beginning of a new closure is to denote that the code within that closure might be run asynchronously with respect to the scope in which it's written. I'm actually a big fan of this rule.

When using ES6 classes it's fairly common to write static getters, especially because there's no syntax specifically for adding a static property to a class. Getters are one of the few types of functions that are synchronous by nature. So, I propose that we introduce a lint rule that allows them to be one-liners, similar to what we allow for arrow functions.

Currently here's what it looks like to add something like a static property to a class,

const Server = class {
    static get framework() {

        return 'hapi';
    }
};

// Server.framework === 'hapi';

I suggest we allow this,

const Server = class {
    static get framework() {
        return 'hapi';
    }
};

// also

const Server = class {
    static get framework() { return 'hapi'; }
};

// Server.framework === 'hapi';

My feeling is that this would alleviate some awkwardness especially for projects within the broader hapi ecosystem that want to adopt the hapi style guide.

Relevant issue, https://github.com/continuationlabs/hapi-scope-start/issues/5

nlf commented 7 years ago

i don't recall it being said that the blank line at the beginning of a function is to denote the possibility of an asynchronous return, where did you read that (i'm curious so we can document it)?

my recollection was that the blank line was simply to help visually distinguish the block from its surrounding code.

i don't really like the idea of making getters a special case, personally. if we're going to propose a change like this i think we should simplify it to allow leaving out the blank line for any function with a single line body.

personally, i simply wouldn't bother with a static getter for your example above, i would do exactly what's in your comment at the bottom:

const Server = class {
    // body
};

Server.framework = 'hapi';

although, i will say that i don't like

const Server = class {
    static get framework() { return 'hapi'; }
};

it just feels wrong to me.

devinivy commented 7 years ago

I was thinking of this comment and conversation, https://github.com/hapijs/contrib/issues/58#issuecomment-151219398. The fact that I feel comfortable pulling getters out as a special case follows very similar reasoning behind arrow functions being allowable (because async + context are not major concerns), and I don't think it extends nicely to all single-line functions.

This works just fine,

const Server = class {
    // body
};

Server.framework = 'hapi';

However, it's not very ergonomic because it's often the case that static props like this are configuration-related, and it's nice to have them as cues near the top of the class.

I wouldn't mind disallowing this,

const Server = class {
    static get framework() { return 'hapi'; }
};

But it might need some special handling compared to the single-line rule for arrows– not 100% certain!