fix(cluster): reorder defaults arguments to prioritize user options

[jstewmon]

Addresses the issue noted in #883 (comment)

[AVVS]

Defaults works the way its intended. From leftmost argument having the highest priority to the rightmost having the least

[jstewmon]

@AVVS I'm not sure what you meant to express.

Yes, defaults takes the value of a given key from the first argument that defines that key. This particular usage of defaults prioritized the Commander constructor's default options over the user-provided options and the cluster-specific defaults. I added a test to demonstrate why that is problematic (it prevents setting showFriendlyErrorStack).

I've just reordered the arguments so that the priority is:

1. user-provided options
2. cluster defaults
3. Commander defaults

Are you suggesting that the existing behavior is correct and the user should not be able to set any options inherited from Commander?

[AVVS]

@jschweik thanks for the details, I did miss that first argument as being passed from the commander, assumed it was from the user input. The change does look good to me now that I've dug into it a little bit more

[ioredis-robot]

🎉 This PR is included in version 4.10.3 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[luin]

Good caught! Btw, is there any particular reasons the former this.options assignment is kept?

[jstewmon]

@luin , no the old assignment should have been removed. I originally commented it out and wrote the new line, so that I could confirm the new test worked as expected, and I accidentally uncommented the old line instead of deleting it. 😞