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

Bug with customDepth when flattening, whereby the customDepth… #47

Merged
merged 1 commit into from
Jun 22, 2016
Merged

Bug with customDepth when flattening, whereby the customDepth… #47

merged 1 commit into from
Jun 22, 2016

Conversation

nickpape
Copy link
Contributor

… value was never decremented/reset. Thus, only the first key was actually being flattened.

Currently, you can easily see this bug if you update the customDepth test to:

flatten({
  hello: {
    world: {
      again: 'good morning'
    }
  },
  lorem: {
    ipsum: {
      dolor: 'good evening'
    }
  }
}, {
  maxDepth: 2
})

Instead of the expected result:

{
  'hello.world': {
    again: 'good morning'
  },
  'lorem.ipsum': {
    dolor: 'good evening'
  }
}

You instead receive:

{
  'hello.world': { 
    again: 'good morning'
  },
  lorem: {
    ipsum: {
      dolor: 'good evening'
    }
  }
}

… value was never decremented/reset. Thus, only the first key was actually being flattened.
@nickpape nickpape changed the title Fixes issue with customDepth when flattening, whereby the customDepth… Bug with customDepth when flattening, whereby the customDepth… May 19, 2016
@nickpape
Copy link
Contributor Author

nickpape commented May 19, 2016

This is a pretty important bugfix IMO, maxDepth is effectively completely broken without it.

@nickpape
Copy link
Contributor Author

Ping @timoxley @hughsk

@timoxley timoxley merged commit 6858dbc into hughsk:master Jun 22, 2016
@timoxley
Copy link
Contributor

Thanks! Good catch, this was indeed a bug.

@timoxley
Copy link
Contributor

Released in 2.0.1

@bendrucker
Copy link
Contributor

Can you publish 2.0.1 to npm?

@timoxley
Copy link
Contributor

@bendrucker Done.

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.

4 participants