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 observer callbacks have wrong this #3343

Open
marcalexiei opened this issue Sep 2, 2020 · 2 comments
Open

Array observer callbacks have wrong this #3343

marcalexiei opened this issue Sep 2, 2020 · 2 comments

Comments

@marcalexiei
Copy link
Member

marcalexiei commented Sep 2, 2020

Description:

During #3334 I noticed that the this set inside array observer callbacks doesn't match the doc.

It seems that this of ObserverArrayCallback should be set to Ractive. See Ractive.d.ts

The problem is inside ArrayObserver which invoke callback without changing the this:
this.callback(this.pending);

To fix the issue I would do something like this:
this.callback.call(this.ractive, this.pending);
however this could result in a breaking change so I would suggest to release change alongside typescript refactor version.

Versions affected:

1.3.x

Platforms affected:

All

Reproduction:

Open this fiddle and check the console:

  • observe function – ArrayObserver
  • observe arrow – Ractive
  • observe as prop – ArrayObserver
const r = window.r = new Ractive({
  el: '#main',
  template: '#template',
  data: {
    array: []
  },
  oninit() {
    this.observe('array', function () {
      console.info('observe function', this);
    }, { array: true });

    this.observe('array', () => {
      console.info('observe arrow', this);
    }, { array: true });
  },

  observe: {
    array: {
      handler() {
        console.info('observe as prop', this);
      },
      array: true,
    }
  }
});
@evs-chris
Copy link
Contributor

Arguably, this is a bug, so it wouldn't be a breaking change. I do have a few bugfixes queued in master, but I've been trying to make as few changes as possible to avoid making uncomfortable diffs here. If it's not bothering anyone, I'm fine with either fixing it now or in the ts branch.

@marcalexiei
Copy link
Member Author

Ok, I'll apply the patch now otherwise I get type error due the typing of ObserverArrayCallback :D

marcalexiei added a commit to marcalexiei/ractive that referenced this issue Sep 3, 2020
marcalexiei added a commit to marcalexiei/ractive that referenced this issue Sep 14, 2020
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