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

<array>.__owl__.parent sometimes refers to the non-observed version of the parent #50

Closed
alexkuhn opened this issue Apr 18, 2019 · 6 comments

Comments

@alexkuhn
Copy link
Contributor

No description provided.

@ged-odoo
Copy link
Contributor

@alexkuhn could you elaborate?

@alexkuhn
Copy link
Contributor Author

Have some crashes because of this, I'm still investigating the issue and will provide short code to reproduce the issue.

@alexkuhn
Copy link
Contributor Author

alexkuhn commented Apr 18, 2019

FYI, following LOC makes it crash with observed array item:
https://github.com/odoo/owl/blob/master/src/observer.ts#L28
It happens after the parent is assigned to another object.

@ged-odoo
Copy link
Contributor

i have a failing test case:

  test("properly handle swapping elements", () => {
    const observer = new Observer();
    const obj: any = { a: { arr: [] }, b: 1 };
    observer.observe(obj);

    // swap a and b
    const b = obj.b;
    obj.b = obj.a;
    obj.a = b;
    expect(observer.rev).toBe(3);

    // push something into array to make sure it works
    obj.b.arr.push("blanche");
    expect(observer.rev).toBe(4);
  });

However, the issue is slightly trickier than I expected. This happens because we unobserve old values when reassigning. It is not hard to make this work for usual situations, but handling cycles in the state is tricky. Not sure yet what to do

1 similar comment
@ged-odoo
Copy link
Contributor

i have a failing test case:

  test("properly handle swapping elements", () => {
    const observer = new Observer();
    const obj: any = { a: { arr: [] }, b: 1 };
    observer.observe(obj);

    // swap a and b
    const b = obj.b;
    obj.b = obj.a;
    obj.a = b;
    expect(observer.rev).toBe(3);

    // push something into array to make sure it works
    obj.b.arr.push("blanche");
    expect(observer.rev).toBe(4);
  });

However, the issue is slightly trickier than I expected. This happens because we unobserve old values when reassigning. It is not hard to make this work for usual situations, but handling cycles in the state is tricky. Not sure yet what to do

@alexkuhn
Copy link
Contributor Author

Shorter test that shows the issue:

test("properly handle assigning observed obj containing array", () => {
    const observer = new Observer();
    const obj: any = { a: { arr: [], val: 'test' } };
    observer.observe(obj);

    obj.a = { ...obj.a, val: 'test2' };
    expect(observer.rev).toBe(2);

    // push something into array to make sure it works
    obj.a.arr.push("blanche");
    expect(observer.rev).toBe(3);
  });

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

2 participants