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

MobX binding fails after deleting property #1730

Closed
1 task done
vitalif opened this issue Sep 20, 2018 · 13 comments
Closed
1 task done

MobX binding fails after deleting property #1730

vitalif opened this issue Sep 20, 2018 · 13 comments

Comments

@vitalif
Copy link

vitalif commented Sep 20, 2018

Hi,

I have an

  1. Issue:

With MobX 5 and React: MobX binding breaks after deleting the property.

The sample is here: https://codesandbox.io/s/7z0pxn9v7j

To reproduce the bug:

  • change the first editbox, for example, to 12. The editbox value will change and you'll see "Modified" below.
  • then change it back to 1. The editbox value will change again and you'll see "Not modified" below.
  • then try to change it to 12 again. You'll see "Modified", but the editbox WON'T CHANGE.

To continue you can click "force update" and it will rerender the row by another property - after that the binding starts to work again.

@urugator
Copy link
Collaborator

Simplified reproduction: https://codesandbox.io/s/n96xrm109l

@leeseean
Copy link

Simplified reproduction: https://codesandbox.io/s/n96xrm109l

I can`t understand your demo

@urugator
Copy link
Collaborator

I can`t understand your demo

  1. Click few times on the first button to see that both observers are reacting properly to o.a modifications
  2. Click the second button to delete prop a from o
  3. Click again the first button to see that the observer subscribed for o.a is no longer reacting...

@leeseean
Copy link

I can`t understand your demo

  1. Click few times on the first button to see that both observers are reacting properly to o.a modifications
  2. Click the second button to delete prop a from o
  3. Click again the first button to see that the observer subscribed for o.a is no longer reacting...

have you compiled the code to es5?

@vitalif
Copy link
Author

vitalif commented Sep 23, 2018

How does it matter?

@mweststrate
Copy link
Member

mweststrate commented Sep 26, 2018

This behavior is working as intended:

  1. delete will set the property to undefined, so that all observers receive an undefined value once, and then throws away the observable. (after all, if we would keep it, we wouldn't delete it and always keep it around in memory, defeating the purpose)
  2. once you reassign to a, it creates a fresh field and observable (in javascript object terms, a new property descriptor is added to the object, the old one isn't revived). In other words, it is a new property, that just happens to have the same name another property had earlier
  3. This is however not a change to the a property, it is a property addition to the o object.
  4. your component is not observing property additions / removals, so it wont know about this new property, ignoring re-renders
  5. you could listen to all property additions / removals, by for example using the keys (or toJS which internally uses keys) utility from mobx, or even for a specific one using has
  6. As is demonstrated in this code sandbox: https://codesandbox.io/s/724zqrxpw1
  7. Which shows that has doesn't react properly, so that actually is a bug, but a different one :-).

EDIT: note that this story only applies to MobX 4!

@urugator
Copy link
Collaborator

urugator commented Sep 26, 2018

@mweststrate

  1. your component is not observing property additions / removals, so it wont know about this new property, ignoring re-renders

How is it then possible that component observes the first addition? Notice that o is initialized empty. On the first addition of prop a, the component is updated. But the consequent additions (after delete) don't work that way...

EDIT: Why should mobx support "up-ahead subscription" via has(o, 'a') (or whatever the syntax is), but not via o.a ? Seems like the same issue to me...

@vitalif
Copy link
Author

vitalif commented Sep 26, 2018

+1. AFAIK that was mobx 4 that didn't handle dynamic objects. From reading the source it seems mobx 5 has all code required to observe property additions and removals.

I also tried to check the dependencies using trace() and they're the same at the beginning and at the end of the test.

@urugator
Copy link
Collaborator

urugator commented Sep 26, 2018

Perhaps it will start working as expected when .has is fixed? Check the get impl:
https://github.com/mobxjs/mobx/blob/master/src/types/dynamicobject.ts#L34

@mweststrate
Copy link
Member

@urugator @vitalif good points, I think I am messing up MobX 4 vs MobX 5 behavior myself now :), will investigate further!

@mweststrate
Copy link
Member

Fixed in MobX 4.5.0 / 5.5.0

@vilicvane
Copy link

@mweststrate I am experiencing a similar issue with mobx 5.5.2:

const {observable, autorun} = require('mobx');

let value = observable({
  foo: undefined, // if foo is something like 'abc', it works.
});

autorun(() => {
  console.log(value.foo);
});

delete value.foo;
value.foo = 'def';

@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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants