ramda/types

[Bug] Error in typing of mergeDeepLeft and mergeDeepRight functions

Closed this issue · 3 comments

alSergey commented

Reproduction

StackBlitz

Expected behavior

No TypeScript errors

Actual behavior

TypeScript Errors

index.ts:10:9 - error TS2322: Type 'MergeDeepObject<Options, Options, BuiltIn, undefined, _OptionalKeys<Options>>' is not assignable to type 'Options'.
  Types of property 'release' are incompatible.
    Type '{ name?: string | undefined; inject?: boolean | undefined; create?: boolean | undefined; finalize?: boolean | undefined; dist?: string | undefined; vcsRemote?: string | undefined; setCommits?: SetCommitsOptions | undefined; deploy?: DeployOptions | undefined; cleanArtifacts?: boolean | undefined; uploadLegacySourcem...' is not assignable to type '{ name?: string | undefined; inject?: boolean | undefined; create?: boolean | undefined; finalize?: boolean | undefined; dist?: string | undefined; vcsRemote?: string | undefined; setCommits?: SetCommitsOptions | undefined; deploy?: DeployOptions | undefined; cleanArtifacts?: boolean | undefined; uploadLegacySourcem...'. Two different types with this name exist, but they are unrelated.
      Type 'MergeDeepObject<{ name?: string | undefined; inject?: boolean | undefined; create?: boolean | undefined; finalize?: boolean | undefined; dist?: string | undefined; vcsRemote?: string | undefined; setCommits?: SetCommitsOptions | undefined; deploy?: DeployOptions | undefined; cleanArtifacts?: boolean | undefined; upl...' is not assignable to type '{ name?: string | undefined; inject?: boolean | undefined; create?: boolean | undefined; finalize?: boolean | undefined; dist?: string | undefined; vcsRemote?: string | undefined; setCommits?: SetCommitsOptions | undefined; deploy?: DeployOptions | undefined; cleanArtifacts?: boolean | undefined; uploadLegacySourcem...'.
        Types of property 'setCommits' are incompatible.
          Type 'SetCommitsOptions | MergeDeepObject<AutoSetCommitsOptions & { previousCommit?: string | undefined; ignoreMissing?: boolean | undefined; ignoreEmpty?: boolean | undefined; }, AutoSetCommitsOptions & { ...; }, BuiltIn, undefined, _OptionalKeys<...>> | MergeDeepObject<...> | MergeDeepObject<...> | MergeDeepObject<...> ...' is not assignable to type 'SetCommitsOptions | undefined'.
            Type 'MergeDeepObject<AutoSetCommitsOptions & { previousCommit?: string | undefined; ignoreMissing?: boolean | undefined; ignoreEmpty?: boolean | undefined; }, ManualSetCommitsOptions & { ...; }, BuiltIn, undefined, _OptionalKeys<...>>' is not assignable to type 'SetCommitsOptions | undefined'.
              Type 'MergeDeepObject<AutoSetCommitsOptions & { previousCommit?: string | undefined; ignoreMissing?: boolean | undefined; ignoreEmpty?: boolean | undefined; }, ManualSetCommitsOptions & { ...; }, BuiltIn, undefined, _OptionalKeys<...>>' is not assignable to type 'AutoSetCommitsOptions & { previousCommit?: string | undefined; ignoreMissing?: boolean | undefined; ignoreEmpty?: boolean | undefined; }'.
                Type 'MergeDeepObject<AutoSetCommitsOptions & { previousCommit?: string | undefined; ignoreMissing?: boolean | undefined; ignoreEmpty?: boolean | undefined; }, ManualSetCommitsOptions & { ...; }, BuiltIn, undefined, _OptionalKeys<...>>' is not assignable to type 'AutoSetCommitsOptions'.
                  Types of property 'repo' are incompatible.
                    Type 'string' is not assignable to type 'undefined'.

10   const optionsLeft: SentryVitePluginOptions = mergeDeepLeft(
           ~~~~~~~~~~~

index.ts:14:9 - error TS2322: Type 'MergeDeepObject<Options, Options, BuiltIn, undefined, _OptionalKeys<Options>>' is not assignable to type 'Options'.

14   const optionsRight: SentryVitePluginOptions = mergeDeepRight(
           ~~~~~~~~~~~~


Found 2 errors in the same file, starting at: index.ts:10
Harris-Miller commented

mergeDeepLeft and mergeDeepRight are not strictly defined by us. We delegate to ts-toolbelt for those

All the merge functions here use O.Assign, which doesn't work for your case. I have no idea why.

There is also a O.Merge type, which does work for you case, however that requires typescript's strictNullChecks, which we couldn't require without a Breaking Change

A special case optimization that I can add is that when the type for l and r are the same, then the return type can just be that type as well, since there is nothing to calculate. That's a simple MR to put up. Will have here in a bit

Demo: https://tsplay.dev/NVdPZW

alSergey commented

Sounds great. I'll be waiting for this change to be released.

alSergey commented

@Harris-Miller hi, can you please tell me if there is any progress on this issue? As I see, the pull request is already ready.