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

Typescript + Mobx observable([]).remove(...) #669

Closed
wildeyes opened this issue Nov 19, 2016 · 13 comments
Closed

Typescript + Mobx observable([]).remove(...) #669

wildeyes opened this issue Nov 19, 2016 · 13 comments

Comments

@wildeyes
Copy link

I am getting a

ERROR in [default] /edit.tsx:80:47
Property 'remove' does not exist on type 'string[]'.

with mobx@2.6.2, webpack with awesome-typescript-loader@2.2.4 and typescript@2.0.8.

@benjamingr
Copy link
Member

benjamingr commented Nov 23, 2016

Can you show a full example I can't reproduce.

@JabX
Copy link

JabX commented Nov 24, 2016

This looks weird, since observable() has no signature that returns a vanilla array.

But if you're using it as a decorator (@observable list = []), then it's to be expected since the decorator doesn't change the type of the property, meaning that even if at runtime list is an ObservableArray (which has the remove() method), at compile time it's still going to be considered as an array. In that case, your only option is an explicit annotation.

@mweststrate
Copy link
Member

Closing for the lack of a reproduction

@mkozhukharenko
Copy link

mkozhukharenko commented Dec 24, 2016

@mweststrate
this example couses an error

class Todo {
  store: TodoStore
  value: string

  constructor(store: TodoStore, value: string) {
    this.store = store
    this.value = value
  }

  @action destroy() {
    this.store.remove(this)
  }
}

class TodoStore {
  title: string = 'TEST'
  @observable todos: Todo[]

  constructor(title: string) {
    this.title = title
    this.todos = [new Todo(this, 'test')] 
  }

  @action removeTodo(todo: Todo) {
    this.todos.remove(todo) // Property 'remove' does not exist on type 'Todo[]'.
  }
}

but if we set todos to IObservableArray everything works

class TodoStore {
  title: string = 'TEST'
  @observable todos: IObservableArray<Todo>

  constructor(title: string) {
    this.title = title
    this.todos = [new Todo(this, 'test')] as IObservableArray<Todo>
  }

  @action removeTodo(todo: Todo) {
    this.todos.remove(todo)
  }
}

Is it ok to write like this?

@JabX
Copy link

JabX commented Dec 25, 2016

Yup that's how you should do it.

But I'd rather do something like this instead:

class TodoStore {
  title: string = 'TEST'
  readonly todos = observable<Todo>([])

  constructor(title: string) {
    this.title = title
    this.todos.push(new Todo(this, 'test'));
  }

  @action removeTodo(todo: Todo) {
    this.todos.remove(todo)
  }
}

This way you have no array annotation and you prevent any array reassignment by marking it readonly (you don't need an @observable decorator if you don't do reassignment). Use ObservableArray.replace() instead if you want to "create" a new array.

@mkozhukharenko
Copy link

@JabX it makes sense.
thank you very much!

@ehartford
Copy link

This goes against the point of the @observable annotation...
If I annotate an array with @observable, typescript should treat it as ObservableArray.
Is there no way to do this with @types/mobx?

@mweststrate
Copy link
Member

@ehartford yes there is since Mobx 3, use

readonly todos = observable.array([])

@JabX
Copy link

JabX commented Feb 2, 2017

An annotation cannot change the type of the property (or class, or method) it is applied to, and I believe it to be the case for all the languages I know of.

@ehartford
Copy link

@JabX The annotation does, in fact, at runtime, change the type from a native array into an ObservableArray. It's just a question of whether TypeScript is aware of the change and able to provide correct dev-time and compile-time validation. If TypeScript doesn't give us the tools to describe the change, then it's a TypeScript bug. If we can describe it but we aren't, it's bug in the MobX typings.

@JabX
Copy link

JabX commented Feb 3, 2017

@ehartford I was talking about compile-time type information, sorry for the inaccuracy. Sure, it does indeed change the type at runtime, which I consider to be a bad practice in general because of the impossibility of describing it at compile-time.

Just don't use the annotation if need the type info, @mweststrate's solution (which is also what I proposed a few posts earlier) is better anyway.

@stiofand
Copy link

stiofand commented Feb 4, 2019

I cant follow this, I'm getting the same error as given above, and I'm simply observing an array of strings:

...
questionQueue = string[];
...
decorators(MyClass, { questionQueue: observable })
Type error: Argument of type '{ questionQueue: IObservableFactory & IObservableFactories & { enhancer: IEnhancer<any>; }; totalQuestionCount: IActionFactory; updateTotalQuestionCount: IActionFactory; question: IActionFactory; ... 6 more ...; resetProgress: IActionFactory; }' is not assignable to parameter of type '{ prototype?: MethodDecorator | PropertyDecorator | MethodDecorator[] | PropertyDecorator[] | undefined; }'.
  Object literal may only specify known properties, and 'questionQueue' does not exist in type '{ prototype?: MethodDecorator | PropertyDecorator | MethodDecorator[] | PropertyDecorator[] | undefined; }'.  TS2345

    52 | 
    53 | decorate(ObservableQuestionStore, {
  > 54 |     questionQueue: observable,

This is just a number of typescript and MObX issues I'm having, when trying to stick to the docs.

@lock
Copy link

lock bot commented Jul 21, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants