Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callable variables and class properties #65

Open
aciccarello opened this issue Sep 19, 2018 · 1 comment
Open

Callable variables and class properties #65

aciccarello opened this issue Sep 19, 2018 · 1 comment
Labels
general discussion Not a bug or enhancement, just a discussion

Comments

@aciccarello
Copy link

An interesting use-case came up in the TypeDoc repo (TypeStrong/typedoc#858). Would it be reasonable to define the following as a function rather than a variable? JSDoc users can use @function but from discussions in #8 I don't know if that would be a good thing to carryover.

/**
 * Promisified version of fs.writeFile().
 * @function writeFileAsync
 */
export const writeFileAsync: Function = util.promisify(fs.writeFile);

A similar use case is instance methods. I know that tslint has made changes to view these the same as methods for the purpose of grouping properties and methods.

class Foo {
  constuctor(events: EventService) {
    events.on('someevent', this.bar);
  }


  /**
   * This acts like a method and can be passed as a callback without losing context of `this`.
   * @param param - new value of prop
   */
  bar = (param: String) => {
    console.log('Am I a method?');
    this.prop = param;
  }
}
@octogonz
Copy link
Collaborator

I recall an office-ui-fabric-react PR #4304 that replaced the @autobind decorator with an incantation similar to what you show above.

I understand why they took that approach, however to me it seemed to have a number of potential downsides as a general practice:

  • It blurs the distinction between code and data, working against the conventional programming language grammar that people rely upon for understanding someone else's source code

  • It may work against optimizations that look for stereotypical patterns (i.e. Webpack, Google Closure, etc may have a harder time proving that it's safe to transform a "member function" that doesn't look like a function)

  • It may not be supported by static analysis tools, e.g. an IDE's features such as "find all callers of this function" or "convert this into a static function" may have trouble determining that this construct should be understood as a function declaration

CC some office-ui-fabric-react people who maybe can provide a counterpoint: @dzearing @cliffkoh @Markionium

@octogonz octogonz added the general discussion Not a bug or enhancement, just a discussion label Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general discussion Not a bug or enhancement, just a discussion
Projects
None yet
Development

No branches or pull requests

2 participants