Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Add a keyof-based inject #615

Closed
mhr3 opened this issue Nov 20, 2018 · 6 comments
Closed

Add a keyof-based inject #615

mhr3 opened this issue Nov 20, 2018 · 6 comments

Comments

@mhr3
Copy link

mhr3 commented Nov 20, 2018

Hi, would it be possible to improve TS bindings for inject, where it wouldn't use simple strings, but template param based keyof instead?

We already have helper interfaces, so it would be nice if these could be passed to inject:

export interface InjectedUserStore {
  userStore: UserStore;
}

@inject<InjectedUserStore>('userStore')
@observer
export class MyComponent extends React.Component<
  Partial<InjectedUserStore> & MyComponentProps
> {
  ...
}

I had a look in mobx sources, and they mentioned some TS decorator bugs, so perhaps this isn't possible atm? But if it is, it would prevent typos when using inject, and provide a tiny bit more type-safety. (of course the string[] based inject could be kept when not using any template params)

@jeremy-coleman
Copy link

jeremy-coleman commented Nov 23, 2018

if you want to prevent typos for inject just use string constants, like var USER_STORE = 'userStore' in some central location, no need for any increased complexity. plus the devtools will warn you immediately if you inject an unavailable store.

@mhr3
Copy link
Author

mhr3 commented Nov 23, 2018

if you want to prevent typos for inject just use string constants, like var USER_STORE = 'userStore' in some central location, no need for any increased complexity. plus the devtools will warn you immediately if you inject an unavailable store.

I would say that is increasing complexity without any real benefit, afterall I already have the interface for the props.

@jeremy-coleman
Copy link

I dont see the point in adding an interface twice to the same component?

@mhr3
Copy link
Author

mhr3 commented Nov 24, 2018

Yes, it would be great if the inject would propagate the prop to the class, but I don't think you can do that with decorators in TS.

@jeremy-coleman
Copy link

I usually just do const {myKeys} = this.props.userStore as UserStore

@mweststrate mweststrate transferred this issue from mobxjs/mobx Nov 30, 2018
@mweststrate
Copy link
Member

Closing for inactivity. See also #256

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

No branches or pull requests

3 participants