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: functions were modifying input options object #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RoystonS
Copy link

The externally-facing looks-same functions were mutating any options object passed into them.

This was problematic because if you wanted to reuse an options object for multiple calls, it would cause failure. For example, if you had an options object that simply had {strict: true}, then during the first call, looks-same would add a threshold property to that, and then complain about the presence of both strict and threshold on the second call.

This PR adds tests to ensure that the options objects are not mutated, and applies two types of fix:

  1. _.defaults mutates its first parameter, so add an extra empty {} as the initial parameter. This way, we accumulate the results in a new object. (It would be even simpler to use an object spread but I don't see any in this codebase and it might cause compatibility issues.)
  2. Don't assign directly to properties on opts, but combine that defaulting with ._defaults calls.

This fixes issue #41

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