selfrefactor / rambda

Faster and smaller alternative to Ramda
https://selfrefactor.github.io/rambda
MIT License
1.65k stars 89 forks source link

types of path are breaking in 7.4.0 #668

Closed Retro64 closed 1 year ago

Retro64 commented 1 year ago

When updating to version 7.4.0 from 7.3.0, the usage of path is breaking. In our code we use the un-curried version:

path<unknown>(['string','array'], object)

I do not see types for an un-curried version using RamdaPath - is this intended?

path<unknown>(['string','array'])(object)

works out.

I would have expected something like

export function path<T>(pathToSearch: RamdaPath, obj: any) => T | undefined;

to be part of the types.

selfrefactor commented 1 year ago

I will have a look and thanks for filling this issue.

selfrefactor commented 1 year ago

Can you provide more detailed example just to be sure that issue is solved? I will restore what you suggest, but you are correct that 7.5.0 make changes to R.path types.

Retro64 commented 1 year ago

I tried do reproduce the error on my private machine, but was only able to reproduce it, if the path did not match the object. So I guess this issue can be closed (and will do this with this comment) - sorry for the false alert @selfrefactor

Retro64 commented 1 year ago

@selfrefactor I finally managed to reproduce it and therefore reopened this issue - I forgot the types on my local machine :

This works out:

import { path } from "rambda";

type UnknownThis = {
    this: unknown
};

const object: UnknownThis = {
    this: {
        is: {
            a: 'path'
        }
    }
};

const test1 = path(['this', 'is', 'not', 'a'])(object);

console.log(test1);

This doesn't:

import { path } from "rambda";

type UnknownThis = {
    this: unknown
};

const object: UnknownThis = {
    this: {
        is: {
            a: 'path'
        }
    }
};

const test1 = path(['this', 'is', 'not', 'a'], object);

console.log(test1);
selfrefactor commented 1 year ago

I don't see it as error:

describe('R.path with array as path', () => {
  interface UnknownThis {
    this: unknown,
  }

  const object: UnknownThis = {
    this: {
      is: {
        a: 'path',
      },
    },
  }
  const test1 = path(['this', 'is', 'not', 'a'])(object)
  test1 // $ExpectType unknown
  const test21 = path(['this', 'is', 'not', 'a'], object)
  test21 // $ExpectType unknown
})

This test passes.

I did test this with 7.2.0 and I didn't see test1 and test21 having different evaluation.

Retro64 commented 1 year ago

May you test with type instead of interface again on 7.4.0? I will also check the used TS version...

selfrefactor commented 1 year ago

interface is lint change; even before that it was the same. It might be from TS version, so let me know if that is the case. I will be closing the issue, but feel free to keep commenting. I cannot reproduce it, so I will close it for the time being.

Retro64 commented 1 year ago

image

package.json

{
  "name": "rambda-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "build": "tsc src/*"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "rambda": "7.4.0",
    "typescript": "4.9.4"
  }
}

tsconfig

{
  "compilerOptions": {
    "target": "es2016",
    "module": "commonjs",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict":true
  }
}
selfrefactor commented 1 year ago

thanks, that helped. It was hard for me to reproduce it, but I finally managed. Fix is applied and it will be released with next version. Thanks for the efforts.

Retro64 commented 1 year ago

Thanks for your fast replies and your dedication. Just gives the feeling we chose the right lib :)

selfrefactor commented 1 year ago

release is made so I am closing this issue