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

fix: narrow the BackoffOptions "type" union #2276

Conversation

lbennett-stacki
Copy link

@lbennett-stacki lbennett-stacki commented Nov 11, 2023

Seems the ability to define custom-named backoff strategies has been dropped, so we can narrow the type type.

Related:
#2275

@lbennett-stacki lbennett-stacki force-pushed the fix/narrow-backoff-options-type-union branch from afae1bf to 9f20bcb Compare November 11, 2023 22:25
@roggervalf
Copy link
Collaborator

hi @lbennett-stacki, we can define custom backoff strategies

@roggervalf
Copy link
Collaborator

@lbennett-stacki
Copy link
Author

lbennett-stacki commented Nov 13, 2023

@roggervalf Thanks, I was getting confused between bull and bullmq custom backoff strategy definitions. I think this is still a valid change that may direct others away from some similar confusion.

In bull you can define a custom backoff strategy in one place and reference it by its unique key in the backoff settings object. However in bullmq you cannot define named/keyed backoff strategies as you did in bull, the value of type in the backoff settings object is now only defined by how it is neither fixed nor exponential, the string value is never used to lookup a strategy. (right???) An explicit "custom" value indicates this better than any string that isn't fixed or exponential IMO.

Let me know if I've misread something!

@roggervalf
Copy link
Collaborator

hey @lbennett-stacki, but custom won't be always a valid value, you are restricting to use fixed, exponential and custom specific values, instead of any other names as custom and ts will detect it as an error if someone define a different custom name apart of 'custom'

@roggervalf
Copy link
Collaborator

as my comment above, this change will cause an error in ts

@roggervalf roggervalf closed this Dec 3, 2023
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.

None yet

2 participants