664153 - GCLI's Object Literal Syntax should be the only syntax

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Joe Walker [:jwalker]]

Mihai:
> - Some of the methods take a wide range of objects as arguments. For example
> canon addCommand() takes an object that is really flexible and I would say
> "prone to errors". "Too much variety" if I may say. I would imagine
> different methods for different syntaxes of adding commands (see the docs
> about the three syntaxes of commands), and more formal definitions of the
> parameters. I mean, for each parameter type you should clearly define the
> expected object properties and so on. Now it all seems like "too flexible"
> and "easy to break". Such flexibility is really good but I believe it might
> not scale in the future.
> 
> A clearer and better defined approach to this stuff would be welcome. (most
> of it might just be a problem of docs actually)

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I tend to agree with Mihai (ish).

I really don't like the Function Metadata Syntax of adding commands. You are changing a function that you possibly don't own (ie the global `alert` from the example) when you could just as easily pass the function in as the exec property. Maybe I am missing something, but isn't

  gcli.addCommand({
    name: 'alert',
    description: '...',
    context: window,
    params: [...],
    exec: window.alert
  });

or even

  gcli.addCommand({
    name: 'alert',
    description: '...',
    params: [...],
    exec: window.alert.bind(window)
  });

effectively the same as

  window.alert.metadata = {
    name: 'alert',
    context: window,
    description: '...',
    params: [...]
  };
  canon.addCommand(window.alert);

?

Messing with functions you don't own seems like poor form to me; especially when there is an equivalent way of doing the same thing that can actually be more concise.

I don't like the Object Syntax that much either. I would rather just use the Object Literal Syntax. It avoids having a giant singleton object which creates all of your commands and sub-commands.

Again, it is mentioned that "it's also possible to decorate a pre-existing object post-creation with metadata". I still don't like changing existing things just to reuse them. Eventually you will get a mess if you keep changing the same object and reusing it over and over. I know that's a "slippery-slope" type argument and that most cases won't be that severe, but this kind of api *does* encourage that kind of misuse. So even though most people won't over do it, this api makes it really easy to. I would prefer to *create* new objects which reuse components of existing objects and functions, than to *mutate* those existing objects and functions (and possibly have collisions, make a big mess, etc). It isn't even that much more concise to do the mutations rather than creation.

I don't think the current api is "too flexible", its just that there are many ways of doing the exact same thing, and none of them really provide more power or different features; just different syntaxes for the same thing. Flexible is good. However, it seems to me that no flexibility is lost by removing 2 of the 3 ways to add commands. But by removing those extraneous ways of adding commands, you lose complexity in the code, which makes it easier to maintain; and you lose complexity in the public api, which makes it easier for developers to learn, understand, and use.

All in all, in my rather opinionated opinion, I would like to have only what is currently dubbed the "Object Literal Syntax" way of adding commands and throw the rest out.

I like that you could have each sub-command in its own module, too, when using the Object Literal Syntax, because each sub-command is separate from each other sub-command. That will undoubtedly be helpful when working on big groups of commands (like git).

Comment 2

[Joe Walker [:jwalker]]

Part of the thinking behinds Function Metadata Syntax was that we might be able to extract metadata from jsdoc comments, so our commands could look like totally normal functions.

Realistically, I don't think this is likely to happen in the short or medium term, and the feature could be added back in without too much difficulty.

In short, I'm convinced lets, change this bug to make Function Metadata Syntax the only syntax.

Comment 3

[Joe Walker [:jwalker]]

C&P error - I mean Object Literal Syntax.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Sounds good to me.

Comment 5

[Joe Walker [:jwalker]]

Marking verified because there is no UI proof that the bug is fixed. The proof is in the code.