Fix compact type for objects

[mathieutu]

Hi,
For now when you pass an object to compact function there is a type problem event if everything is good.

This PR fix this behaviour.

This PR fix another issue with compact, so I didn't do anything which could collide with it.
#1603

Thanks for the job.

[char0n]

@mathieutu thanks for bringing this up. Though I agree with the premise of this function supporting objects (which it does), we have do a little bit more then just adding overloaded type def.

IMHO we have to add additional sentence: Filterable objects include plain objects or any object that has a filter method such as Array.. Along with that we have to change description in source file, add new examples and incorporate new tests.

Would you like to take this over, or should I create a separate ticket for this ?

[mathieutu]

Hi @char0n
I agree with all of that.

I think we could iterate on it : merging this one to fix the type issue to begin, and making a ticket to handle the other changes afterwards.

[char0n]

@mathieutu well along with typing you also changed the description of the function.

If you amend the PR and return the description to original state I'll be willing to just change the typing to add support for Object. If we want to change the description also we'll have to proceed as I suggested.

[char0n]

On another note, what is the difference between TS Dictionary and Record? Why prefer the Dictionary? Thanks for enlightening me ;]

[mathieutu]

It's done.

Dictionnary is an internal type of ramda adjunct, it's why I've used it:

    interface Dictionary<T> { [key: string]: T; }

It's exactly the same than Record<string, T>.

[mathieutu]

I've updated the type to follow #1603 changes, and improved it in cas of complex objects. It return the same keys but without falsy values.

[char0n]

The change looks fine but we're not failing a typing test. More info here: https://app.circleci.com/pipelines/github/char0n/ramda-adjunct/741/workflows/ef5169e4-93c7-44f6-81b1-d58c0806c5b1/jobs/21335/parallel-runs/0/steps/0-103

[BenoitHiller]

Sticking my head in here because my PR got mentioned and I see this is still open.

With the latest changes here you should just delete this line: https://github.com/char0n/ramda-adjunct/pull/1611/files#diff-88bec6beae84369c0376b56b3bb88fe1R1190

Arrays are objects and so the index signature you have there works just as well for them.

The two failing tests are https://github.com/char0n/ramda-adjunct/blob/master/types/test/compact.ts#L15 and https://github.com/char0n/ramda-adjunct/blob/master/types/test/compact.ts#L18. Your change makes them no longer correct so you should just delete them (npm run test:types to run the type tests. this isn't documented anywhere).

I personally would also add in some tests for filtering objects as it gets pretty complicated sometimes.

[char0n]

@mathieutu are you willing to continue on this PR as @BenoitHiller suggested?