http-assert icon indicating copy to clipboard operation
http-assert copied to clipboard

Remove deep-equal dependency

Open mehulkar opened this issue 1 year ago • 8 comments

Hello!

I'm taking a stab at some "ecosystem cleanup" shepherded by someone else[^1]. One of those issues is to remove deep-equal from this library as it has 18 dependencies.

I see from #15 and #14 that dequal isn't a good enough replacement, but would be interested in finding an alternative if the maintainers are interested in "solving" this issue.

I know that reducing dependencies in one library doesn't "boil the ocean", so to speak, but I wanted to start somewhere, and this happened to be where I started. If you see value any value in this sort of work and would accept PRs toward this goal, please let me know! The thing I'd need is your expertise / experience to fill in the gaps that the test suite doesn't. Thank you in advance!

[^1]: not tagging, b/c the reason for me taking it on is to help spread the work

mehulkar avatar Jan 29 '24 04:01 mehulkar

Hello, I have no issue with replacing it with a better module, but it just needs to actually do the Node.js deep equal algorithm it is put there in this module in order to provide.

dougwilson avatar Jan 29 '24 04:01 dougwilson

Any chance you can give me a single test case or loose example that would demonstrate such a deep equal that is relevant for http-assert? A pointer to any of deep-equal's source/tests that are relevant would be useful also!

mehulkar avatar Jan 29 '24 04:01 mehulkar

Hello, I am sorry, but I won't have time to do that for a while, as I have much higher priority objectives than this. If you are interested in this, I think you can easily find the test suite for deep-equal in the module and there you go, that's what whatever you want to replace it with should do.

dougwilson avatar Jan 29 '24 04:01 dougwilson

The simple answer is that deepEqual is a == not strict deep equals, but the module you proposed, dequal is completely the opposite, using ===, or strict equality. That is a completely different way to compare so definitely not a valid replacement option.

dougwilson avatar Jan 29 '24 04:01 dougwilson

@dougwilson my suggestion is that you use deep-eql and pass a comparator for your deepEqual function

i.e. your strict function would call deepEqual(a, b) (from deep-eql)

your non-strict would call deepEqual(a, b, {comparator})

then your comparator would be like so:

deepEqual(left, right, {
  comparator: (a, b) => {
    if(typeof a === 'object' || typeof b === 'object') {
      return null; // this tells deep-eql to carry on using default behaviour
    }
    return a == b;
  }
});

happy to make a branch with that unless @mehulkar beats me to it 👀

43081j avatar Mar 29 '24 15:03 43081j

Actually, I am no longer involved in this project, so up to whoever is planning to take it over. I am not even a committer these days :)

dougwilson avatar Mar 29 '24 15:03 dougwilson

if you need someone to take over, i'd be happy to help if there's still need/usage for the project

let me know. would be up for getting this change in too and maybe some dependency updates

43081j avatar Mar 29 '24 17:03 43081j

I'm not a committer any more, so cannot make such decisions. Just continue what you like and whoever comes along to pick up the project will see. No need to at message me, however, since nothing I can do.

dougwilson avatar Mar 29 '24 17:03 dougwilson