Skip to content

Conversation

@analog-nico
Copy link

As discussed in issue #12 [email protected] introduced a memory leak by no longer aborting all requests – even those which successfully finished. This created a memory leak because this.abortControllers slowly filled up with the AbortController instances of queries that finished successfully and were not aborted.

I decided to not simply reintroduce the pre-1.5.7 code because the change was technically a breaking change: Previously, users could expect that the 'abort' event got always fired – no matter if the query got aborted or if it finished. Since passing a custom abortController is part of the library’s API, the change in behavior created a breaking change for users. Of course, the change was accidental and the increased patch version number did not reflect the breaking change.

Since most users are using @1.6.0, I decided that the 'abort' event shall only be sent if the query is actually aborted. This is in line with all versions from @1.5.7 forward – keeping the behavior that most users expect nowadays. (And it makes logically more sense.) If you publish my changes then I recommend using version 1.6.1.

Unfortunately, I was not able to run the tests on my system. For some reasons all tests failed even without my changes. Could you do the quality assurance on your side since you know the dev env of your library much better than me?

I can at least say that with careful code review I made the code changes in a way that it is guaranteed that the AbortController instances are always removed from this.abortControllers while any impact on the existing logic is impossible by working with try ... catch and try ... finally blocks.

@titanism
Copy link
Contributor

Is this ready yet? Thank you!

@analog-nico
Copy link
Author

Yes, it is. Sorry, I had to add a second commit but that’s it.

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

Successfully merging this pull request may close these issues.

2 participants