Fix support for rxjs@>=7

[skrtheboss]

Fix some issues with latest version of npm packages.

[shlomiassaf]

I got a new Mac. I need to setup a lot of stuff

Once I'm done I'll dive into it.

Thanks!

[rubiesonthesky]

This would super nice to have soon! ngrid is blocking Angular 13 update right now for us. :)

[shlomiassaf]

Will review next week

[shlomiassaf]

@skrtheboss Sorry for the long delay.

So, i'm trying to review this one, but there are 2 .ts files with lot's of changes that seems to be automated linter code styling changes which makes it super hard to review.

It might be that there are no code changes there at all, if that's the case can you omit those from the commit?

[shlomiassaf]

Ok, I understand a bit more.

This is actually 3 PR's in one.

1) Allow more loose dependency versioning schema 2) Support invalid type declaration (due to lib DOM updates) 3) Fix breaking changes in RXJS 7

First, for point (2) - No need, it will go in as part of #220

For (1) - That's great

For (3)

This is a breaking change here as well break anyone who uses RxJs 6.5+ which angular supports (v14 at the time this is written)

So it's an issue here which needs to be handled with backward compatibility in mind.

Best option is to detect the RX version. Either it's provided or otherwise hard-code it in the build process.

The other option is to accept a parameter, which I would like to avoid.

[shlomiassaf]

@skrtheboss

In version 5.0.0 all of the issues are resolved except the RXJS issue.

i've ported to rxjs and ran it, did not find any issues, I've also added your code which actually changed the behavior as it was with rxjs 6, without your code rxjs 7 yields the same behavior as 6.

So, I could not reason this issue.

Please, if possible, reproduce in StackBlitz

Thanks!

[skrtheboss]

Hi @shlomiassaf, thank you for reviewing the pull request.

I do not remember why i had issues with rxjs, but as my commit states,

Since rxjs 7.0.0-beta.11 https://github.com/ReactiveX/rxjs/blob/master/CHANGELOG.md#breaking-changes-2, the buffer also emits the buffer on notifier completion. So we need to skip the last value emitted by the buffer.

they have made some changes to the buffer operator, so if you can double check and it does not make any difference, than I am okay to close this pull request. And will open a new one if the problem still persists.

Thank you!

[skrtheboss]

I trried out the latest version, and i get an error printed in the consoloe:

ERROR TypeError: Cannot read properties of undefined (reading 'prev') at pebula-ngrid.mjs:3929:117 at map.js:7:37 at OperatorSubscriber._next (OperatorSubscriber.js:13:21) at OperatorSubscriber.next (Subscriber.js:31:18) at buffer.js:8:24 at OperatorSubscriber._complete (OperatorSubscriber.js:36:21) at OperatorSubscriber.complete (Subscriber.js:49:18) at Subject.js:59:39 at errorContext (errorContext.js:19:9)

pebula-ngrid.mjs:3929:

this.focusChanged = this.focusChanged$ .pipe(buffer(this.focusChanged$.pipe(debounceTime(0, asapScheduler))), map(events => ({ prev: events[0].prev, curr: events[events.length - 1].curr })));

As far as i understand events[0] is undefined, and this is probably caused by the buffer operator changes.

[shlomiassaf]

@skrtheboss I understand.

I could not reproduce it.

I would like to fix it but I must also verify it works with RXJS 6+ as it is officially supported by angular.
For that I need you to reproduce the issue in StackBlitz

Also, the commit has many style changes applied automatically by your editor I guess. If you can remove them it will be helpful.

Thanks!

[jcachat]

This is blocking me too. The error happens when the grid is being destroyed when you navigate to a new page. Here is a stackblitz to demonstrate: https://stackblitz.com/edit/pebula-ngrid-starter-cjyb7v

The grid is empty but that does not affect anything - error happens no matter what.

[shlomiassaf]

This is blocking me too. The error happens when the grid is being destroyed when you navigate to a new page. Here is a stackblitz to demonstrate: https://stackblitz.com/edit/pebula-ngrid-starter-cjyb7v

The grid is empty but that does not affect anything - error happens no matter what.

Thanks, now I have something to work with

[rubiesonthesky]

I trried out the latest version, and i get an error printed in the consoloe:

ERROR TypeError: Cannot read properties of undefined (reading 'prev') at pebula-ngrid.mjs:3929:117 at map.js:7:37 at OperatorSubscriber._next (OperatorSubscriber.js:13:21) at OperatorSubscriber.next (Subscriber.js:31:18) at buffer.js:8:24 at OperatorSubscriber._complete (OperatorSubscriber.js:36:21) at OperatorSubscriber.complete (Subscriber.js:49:18) at Subject.js:59:39 at errorContext (errorContext.js:19:9)

pebula-ngrid.mjs:3929:

this.focusChanged = this.focusChanged$ .pipe(buffer(this.focusChanged$.pipe(debounceTime(0, asapScheduler))), map(events => ({ prev: events[0].prev, curr: events[events.length - 1].curr })));

As far as i understand events[0] is undefined, and this is probably caused by the buffer operator changes.

This is same error as in this issue https://github.com/shlomiassaf/ngrid/issues/215 - It seems it has been there since Angular 12. But somehow now Angular 14 & rxjs 7 is causing it to appear to me too.

I think it has something to do with angular page routing, because it's now manifesting when I click link in table to go to another page. But since some people where having the problem already with Angular 12, I don't think it's purely problem with rxjs 7.