Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

fix(merge): consider "null" as not an object #709

Closed
wants to merge 3 commits into from
Closed

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented May 15, 2019

This behaves the same as lodash (but they didn't have a test for it)

This fixes the behaviour seen in http://localhost:6006/?path=/story/breadcrumb--with-hierarchical-menu on refactor/usage-search-parameters (on InstantSearch.js when you link this version of the helper)

IFW-777

This behaves the same as lodash (but they didn't have a test for it)
test/spec/functions/merge.js Outdated Show resolved Hide resolved
@Haroenv Haroenv requested review from samouss and a team May 15, 2019 09:13
@ghost ghost requested review from tkrugg and removed request for a team May 15, 2019 09:13
src/functions/merge.js Outdated Show resolved Hide resolved
src/functions/merge.js Outdated Show resolved Hide resolved
Copy link
Contributor

@samouss samouss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does solve the issue with null, but I've tried with InstantSearch and the HierarchicalMenu does not work properly. Multiple levels are opened at the same time with duplicated values. It looks like we share a reference somewhere. I've tried with the merge function of Lodash and the problem is gone.

Screenshot 2019-05-15 at 18 08 51

@Haroenv Haroenv closed this Jun 24, 2019
@Haroenv Haroenv deleted the fix/merge-null branch June 24, 2019 09:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants