Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Struct.get #1890

Closed
lo1tuma opened this issue Jan 9, 2024 · 8 comments · Fixed by #1891
Closed

Implement Struct.get #1890

lo1tuma opened this issue Jan 9, 2024 · 8 comments · Fixed by #1891
Assignees
Labels
enhancement New feature or request

Comments

@lo1tuma
Copy link

lo1tuma commented Jan 9, 2024

What is the problem this feature would solve?

Similar to Struct.pick it would be nice to have a util function that can extract the value of a single property within the given object. This is useful when iterating over an array of objects in order to extract just one property, for example:

const users = [ { name: 'Bob', age: 42 }, { name: 'Alice', age: 21 } ];
const getName = Struct.get('name');
const allNames = users.map(getName) // [ 'Bob', 'Alice' ]

What is the feature you are proposing to solve the problem?

The implementation could look like this

function get<T extends string>(key: T): <S extends Record<T, unknown>>(s: S) => S[T] {
    return (s) => {
        return s[key];
    };
}

Example Usage:

const getAge = get('age');

type User = {
    age: number;
    name: string;
};
const user: User = { name: 'Bob', age: 42 };
const result = getAge(user); // number

type JobDescription = {
    title: string;
};
const job: JobDescription = { title: 'Sales-Manager' };
const result2= getAge(job); // type error

What alternatives have you considered?

No response

@lo1tuma lo1tuma added the enhancement New feature or request label Jan 9, 2024
@gcanti
Copy link
Contributor

gcanti commented Jan 10, 2024

IME it's not easy to get the typing right:

function get<T extends string>(key: T): <S extends Record<T, unknown>>(s: S) => S[T] {
  return (s) => {
    return s[key]
  }
}

const record: Record<string, number> = {}

// const value: number
const value = pipe(record, get("a"))

@lo1tuma
Copy link
Author

lo1tuma commented Jan 10, 2024

Good point, I haven’t thought about this. But I guess from a typing perspective it would be solvable to detect if the key matches the key of an index signature:

function get<T extends string>(
    key: T
): <S extends Record<T, unknown>>(s: S) => {} extends Record<keyof S, unknown> ? S[T] | undefined : S[T] {
    return (s) => {
        return s[key];
    };
}

const record: Record<string, number> = {}

// const value: number | undefined
const value = pipe(record, get("a"))

But that requires us to deal with possible undefined values.


FWIW: I’ve checked how Struct.pick and Schema.pick handles this.

Regarding Struct.pick it avoids the problem by forcing the user the specify the type of the object when the combinator is created, which makes it hard to safe the combinator in a variable in order to re-use later.
Regarding Schema.pick it seems to suffer from the same problem, see:

import * as S from '@effect/schema/Schema';
const mySchema = S.record(S.string, S.number);
const onlyAgeSchema = mySchema.pipe(S.pick('age'));
type OnlyAge = S.Schema.To<typeof onlyAgeSchema> // type { age: number }

@gcanti
Copy link
Contributor

gcanti commented Jan 10, 2024

Regarding Struct.pick it avoids the problem...

actually it seems that Struct.pick has the same issue:

import { pipe } from "effect/Function"
import * as Struct from "effect/Struct"

const record: Record<string, number> = {}

/*
const value: {
    a: number;
    b: number;
}
*/
const value = pipe(record, Struct.pick("a", "b"))

@gcanti gcanti self-assigned this Jan 10, 2024
gcanti added a commit that referenced this issue Jan 10, 2024
@gcanti
Copy link
Contributor

gcanti commented Jan 10, 2024

@lo1tuma I got get implemented and a couple of fix here #1891
Working on a fix for Schema.pick...

@lo1tuma
Copy link
Author

lo1tuma commented Jan 10, 2024

Wow, that was fast. Thank you for working on this 🙇.

@gcanti
Copy link
Contributor

gcanti commented Jan 10, 2024

Regarding Schema.pick, I think the issue is the opposite, considering that the purpose of schema APIs is to adhere as closely as possible to the behavior of the built-in Pick type. It is the runtime that needs to adapt, so the fix is to ensure that the returned schema is consistent with the returned type.

So this is ok

import * as S from "@effect/schema/Schema"

const schema = S.record(S.string, S.number)

const OnlyAge = schema.pipe(S.pick("age"))

type OnlyAge = S.Schema.To<typeof OnlyAge> // type { age: number }

but the current produced AST is wrong

console.log(OnlyAge.ast) // wrong AST
/*
{
  _tag: 'TypeLiteral',
  propertySignatures: [],
  indexSignatures: [],
  annotations: {}
}
*/

should be

{
  _tag: 'TypeLiteral',
  propertySignatures: [
    {
      name: 'age',
      type: {
        _tag: 'NumberKeyword',
        annotations: {
          [Symbol(@effect/schema/annotation/Title)]: 'number',
          [Symbol(@effect/schema/annotation/Description)]: 'a number'
        }
      },
      isOptional: false,
      isReadonly: false,
      annotations: {}
    },
    [length]: 1
  ],
  indexSignatures: [ [length]: 0 ],
  annotations: {}
}

@gcanti
Copy link
Contributor

gcanti commented Jan 10, 2024

btw curious that despite starting from a readonly record a mutable type is returned:

type A = Pick<{ readonly [x: string]: number }, "age">
/*
type A = {
  age: number;
}
*/

gcanti added a commit that referenced this issue Jan 10, 2024
@lo1tuma
Copy link
Author

lo1tuma commented Jan 10, 2024

Regarding Schema.pick, I think the issue is the opposite, considering that the purpose of schema APIs is to adhere as closely as possible to the behavior of the built-in Pick type

In this case you are right, the schema is behaving correctly. IMHO the behavior of the built-in Pick type already bad especially when noUncheckedIndexedAccess is enabled, but that’s nothing we can change unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants