Skip to content

chore: add safeMerge shared util - #166

Open
taran-a wants to merge 1 commit into
mainfrom
chore/add-safeMerge-shared-util
Open

chore: add safeMerge shared util#166
taran-a wants to merge 1 commit into
mainfrom
chore/add-safeMerge-shared-util

Conversation

@taran-a

@taran-a taran-a commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Explanation

Add safeMerge shared util.
Use it in tron and solana snaps.
Add two test cases.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

@taran-a
taran-a requested review from a team as code owners August 18, 2026 17:52
@taran-a
taran-a deployed to default-branch August 18, 2026 17:52 — with GitHub Actions Active
RemoteFeatureFlagsProvider,
type RemoteFeatureFlagsProviderMessenger,
} from './providers/remote-feature-flags/RemoteFeatureFlagsProvider';
export { safeMerge } from './safeMerge';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not related to this PR

but can we add export of logger too? it seem missing?

* const overridee = { name: 'John' };
* const overrider = { name: undefined, age: 30 };
* const merged = safeMerge(overridee, overrider);
* // merged is { name: 'John' }

@stanleyyconsensys stanleyyconsensys Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Safe megre is actually:

Not a deep merge (only one level)
BigNumber is not support (it is fine, but since we use bignumber rather than bigint)
not support Date, Map, Set, empty arrays <-- empty array is more important

consider using lodash with a wrapper to handle null case

something like

import { isPlainObject, mergeWith } from 'lodash';
export const safeMerge = <TOverridee extends object, TOverrider extends object>(
  overridee: TOverridee,
  overrider: TOverrider,
): TOverridee & TOverrider=>
  mergeWith({}, overridee, overrider, (objValue, srcValue) => {
    // handle null case
    if (srcValue === undefined || srcValue === null) {
      return objValue;
    }
    // handle {}
    if (isPlainObject(srcValue) && Object.keys(srcValue).length === 0) {
      return objValue;
    }
    if (!isPlainObject(objValue) || !isPlainObject(srcValue)) {
      return srcValue;
    }
    // let lodash recurse
    return undefined; 
  });

but if we just wanna to limit up our usecase, it also fine, but worth to mention all limitation in the jsdoc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants