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

Feature: Update @computedFrom to support typing #727

Open
silbinarywolf opened this issue Nov 26, 2018 · 9 comments
Open

Feature: Update @computedFrom to support typing #727

silbinarywolf opened this issue Nov 26, 2018 · 9 comments

Comments

@silbinarywolf
Copy link

silbinarywolf commented Nov 26, 2018

I'm submitting a feature request

  • Library Version:
    1.7.3

Please tell us about your environment:

  • Operating System:
    Windows 10

  • Node Version:
    v8.11.1

  • Yarn Version:
    1.5.1

  • Language:
    TypeScript 3.0

Current behavior:
We do not get any type safety when using @computedFrom. A few blogs give users a method for extending @computedFrom manually, but why not just make this an upstream feature we can use?

What is the expected behavior?
That @computedFrom can give us type safety out-of-the-box.

https://medium.com/tech-effectory/creating-a-typed-version-of-aurelias-computedfrom-decorator-with-typescript-27219651ecee

Example:

import {computedFrom as originalComputedFrom} from "aurelia-framework"; 
export function computedFrom<T>(...rest: Array<keyof T>) { 
  return originalComputedFrom(...rest); 
}
import {computedFrom} from './typedcomputedfrom'; 
export class App { 
	public foo = "a"; 
	public bar = "b";

	@computedFrom<App>("foo") 
	public get foobar () { 
		return `${this.foo} ${this.bar}`; 
	} 
}

What is the motivation / use case for changing the behavior?

  • More bugs can be caught at compile-time, this will reduce chance of "typo" mistakes when using @computedFrom
  • Rather than individual projects adopt this pattern and import a "fake" version of @computedFrom across their codebase, it's standard.
@silbinarywolf silbinarywolf changed the title Feature: Feature: Update @computedFrom to support typing Nov 26, 2018
@bigopon
Copy link
Member

bigopon commented Nov 26, 2018

@silbinarywolf Awesome issue. It would be nice if you could help update the typings here https://github.com/aurelia/binding/blob/master/src/aurelia-binding.d.ts#L877

@silbinarywolf
Copy link
Author

silbinarywolf commented Nov 27, 2018

I experimented locally with putting this in as so:
node_modules\aurelia-binding\dist\aurelia-binding.d.ts

/**
* Decorator: Indicates that the decorated property is computed from other properties.
* @param propertyNames The names of the properties the decorated property is computed from.  Simple property names, not expressions.
*/
export declare function computedFrom(...propertyNames: string[]): any;

/**
* Decorator: Indicates that the decorated property is computed from other properties. Adds compile-time type safety to @computedFrom.
* @param propertyNames The names of the properties the decorated property is computed from.  Simple property names, not expressions.
*/
export declare function computedFrom<T>(...propertyNames: Array<keyof T>);

However, what I found was that keyof doesn't work with private members. So now Im not so sure on this feature as I wouldn't want inexperienced programmers to think they should make various members "public" because that gives less errors.

@silbinarywolf
Copy link
Author

There's a thread discussing the problem in detail here:
microsoft/TypeScript#13543

@bigopon
Copy link
Member

bigopon commented Nov 27, 2018

Oh nice find. We can come back to this later. One thing we need is to ensure backward compatibility, so it would be

export declare function computedFrom<T = any>(...propertyNames: Array<keyof T>);

@silbinarywolf
Copy link
Author

Well, the way I've posted above is backwards compatible as well, it just has two-definitions, one without the polymorphic parameter and one with it :)

If your version has no backwards compatibility breaks, then I'd go with that :)

@bigopon
Copy link
Member

bigopon commented Nov 27, 2018

Nice. I didn't notice that. Thought it was for diffing

@qraynaud
Copy link

qraynaud commented Sep 8, 2019

I also want to point out that computedFrom also supports sub properties like @computedFrom('sub.value'). It should be supported by the typing if you want to go that way.

@AnorZaken
Copy link

AnorZaken commented Jul 4, 2022

The problem with private members can be solved by using an expression instead of keyof.
E.g.

class Example {
  private secret: string;
  @computedFrom<Example>(x => x.hidden)
  public get wisperedSecret(): string { return `Psst! ${this.secret}`; }
}

As long as the expression is typed inside the scope of the class itself, like above, you can access private members as expected.

I'm already using something similar today, where I use a Proxy<> to walk the expression chain to turn it into a string, so it also works with x => x.foo.bar.baz for example.

@anton-gustafsson
Copy link
Sponsor

@AnorZaken Looks great! 🚀 Do you mind sharing the solution using Proxy<>?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants