bbc / sqs-consumer

Build Amazon Simple Queue Service (SQS) based applications without the boilerplate
https://bbc.github.io/sqs-consumer/
Other
1.74k stars 333 forks source link

fix: removing bind and refactoring consumer #478

Closed mogu4iy closed 6 months ago

mogu4iy commented 6 months ago

Resolves #NUMBER

Description:

There is an inheritance issue, that it's not possible to extend Consumer class. It happens because you autoBind all propertyNames from constructor.prototype, so while extending these properties are not accessible anymore in this way.

Type of change:

Why is this change required?:

There are cases when we what to extend Consumer class, to pass pre defined options for example. I have created minimal reproducible exaple.

Code changes:


Checklist:

mogu4iy commented 6 months ago

@nicholasgriffintn I hope you can check this as well ;)

nicholasgriffintn commented 6 months ago

Hey, thanks for the PR.

I'm not too sure the reasoning behind the bind, and if we need it or not honestly, it's before my time.

I'll have to have a look through this when I have a spare hour or two, bare with :).

mogu4iy commented 6 months ago

Hey, thanks for the PR.

I'm not too sure the reasoning behind the bind, and if we need it or not honestly, it's before my time.

I'll have to have a look through this when I have a spare hour or two, bare with :).

It was used to avoid passing class methods to intervals or promise then calls using arrow functions.

nicholasgriffintn commented 6 months ago

Yeh it was part of the original commit years ago, so potentially just old Node/JS thoughts. I'll have a proper look through tomorrow.

codeclimate[bot] commented 6 months ago

Code Climate has analyzed commit 3a292f23 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 98.3% (0.0% change).

View more on Code Climate.

mogu4iy commented 6 months ago

@nicholasgriffintn any updates?

mogu4iy commented 6 months ago

I am also interested in estimates, when you are going to release 10.0.0