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 at method for tuples #2976

Merged
merged 9 commits into from
Jun 14, 2024

Conversation

mvares
Copy link
Contributor

@mvares mvares commented Jun 10, 2024

Closes #2975

@mvares mvares requested a review from mikearnaldi as a code owner June 10, 2024 17:30
Copy link

changeset-bot bot commented Jun 10, 2024

🦋 Changeset detected

Latest commit: 7d9686e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 25 packages
Name Type
effect Minor
@effect/cli Major
@effect/experimental Major
@effect/opentelemetry Major
@effect/platform-browser Major
@effect/platform-bun Major
@effect/platform-node-shared Major
@effect/platform-node Major
@effect/platform Major
@effect/printer-ansi Major
@effect/printer Major
@effect/rpc-http Major
@effect/rpc Major
@effect/schema Major
@effect/sql-drizzle Major
@effect/sql-mssql Major
@effect/sql-mysql2 Major
@effect/sql-pg Major
@effect/sql-sqlite-bun Major
@effect/sql-sqlite-node Major
@effect/sql-sqlite-react-native Major
@effect/sql-sqlite-wasm Major
@effect/sql Major
@effect/typeclass Major
@effect/vitest Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mvares mvares changed the title Implement at method for tuples Implement at method for tuples Jun 10, 2024
@github-actions github-actions bot changed the base branch from main to next-minor June 10, 2024 17:30
@mikearnaldi
Copy link
Member

mikearnaldi commented Jun 10, 2024

We could probably do better by using N extends number instead of number so that T[N] is the actual tuple type at N

@mvares
Copy link
Contributor Author

mvares commented Jun 10, 2024

We could probably do better by using N extends number instead of number so that T[N] is the actual tuple type at N

Ready!

@mikearnaldi
Copy link
Member

export const at: {
  <A extends ReadonlyArray<unknown>, N extends number>(index: N): (self: A) => A[N]
  <A extends ReadonlyArray<unknown>, N extends number>(self: A, index: N): A[N]
}

wondering if N should instead extends keyof A, if instead we leave it as number then it would allow undefined access to properties not in the tuple (which may / may not be ok) but in that case a more correct signature would be:

export const at: {
  <N extends number>(index: N): <A extends ReadonlyArray<unknown>>(self: A) => A[N]
  <A extends ReadonlyArray<unknown>, N extends number>(self: A, index: N): A[N]
}

So that it allows at(n) to be used as a function even outside of a pipe.

cc @gcanti @tim-smart

@gcanti
Copy link
Contributor

gcanti commented Jun 11, 2024

I would mirror what we did in Struct.ts:

src/Tuple.ts

export const get =
  <I extends number>(index: I) => <T extends ReadonlyArray<any>>(t: T): [] extends T ? T[I] | undefined : T[I] =>
    t[index]

dtslint/Tuple.ts

import { hole, pipe } from "effect/Function"

// -------------------------------------------------------------------------------------
// get
// -------------------------------------------------------------------------------------

// $ExpectType <T extends readonly any[]>(t: T) => [] extends T ? T[0] | undefined : T[0]
const get0 = get(0)

// $ExpectType undefined
pipe(hole<[]>(), get0)

// $ExpectType undefined
pipe(hole<readonly []>(), get0)

// $ExpectType string
pipe(hole<[string, number]>(), get0)

// $ExpectType string
pipe(hole<readonly [string, number]>(), get0)

// $ExpectType number
pipe(hole<[string, number]>(), get(1))

// $ExpectType number
pipe(hole<readonly [string, number]>(), get(1))

// $ExpectType undefined
pipe(hole<[string, number]>(), get(2))

// $ExpectType undefined
pipe(hole<readonly [string, number]>(), get(2))

// $ExpectType string | number
pipe(hole<[string, number]>(), get(-1))

// $ExpectType string | number
pipe(hole<readonly [string, number]>(), get(-1))

// $ExpectType number | undefined
pipe(hole<Array<number>>(), get(1))

// $ExpectType number | undefined
pipe(hole<Array<number>>(), get(-1))

@mvares
Copy link
Contributor Author

mvares commented Jun 11, 2024

thank you @gcanti! what do you think, @mikearnaldi?

@mvares
Copy link
Contributor Author

mvares commented Jun 11, 2024

It would be something as:

/**
 * Retrieves the element at a specified index from a tuple.
 *
 * @param self - A tuple from which to retrieve the element.
 * @param index - The index of the element to retrieve.
 *
 * @example
 * import { at } from "effect/Tuple"
 *
 * assert.deepStrictEqual(at([1, 'hello', true], 1), 'hello')
 *
 * @category getters
 * @since 3.4.0
 */
export const at: {
  <N extends number>(index: N): <A extends ReadonlyArray<unknown>>(self: A) => A[N]
  <A extends ReadonlyArray<unknown>, N extends number>(self: A, index: N): A[N]
} = dual(2, <A extends ReadonlyArray<unknown>, N extends number>(self: A, index: N): A[N] => self[index])

@mikearnaldi
Copy link
Member

I think I am ok with it, it's in line with schema

@mvares
Copy link
Contributor Author

mvares commented Jun 11, 2024

I will send a PR to add it

@mvares
Copy link
Contributor Author

mvares commented Jun 11, 2024

cc @mikearnaldi 👍🏻

@github-actions github-actions bot force-pushed the f/add-at-method-for-tuples branch 2 times, most recently from 5a5e747 to 5fd0054 Compare June 12, 2024 01:10
@mvares mvares force-pushed the f/add-at-method-for-tuples branch from 5fd0054 to 57465f3 Compare June 12, 2024 01:26
@mvares mvares requested a review from tim-smart June 12, 2024 01:29
@tim-smart
Copy link
Member

I would be good to add the dtslint tests @gcanti quoted above.

github-actions bot pushed a commit that referenced this pull request Jun 15, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 16, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 16, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 16, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 17, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 17, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 17, 2024
Co-authored-by: Tim <hello@timsmart.co>
@mvares mvares deleted the f/add-at-method-for-tuples branch June 17, 2024 10:40
github-actions bot pushed a commit that referenced this pull request Jun 17, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 17, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 17, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 19, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 19, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 19, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
github-actions bot pushed a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
tim-smart added a commit that referenced this pull request Jun 20, 2024
Co-authored-by: Tim <hello@timsmart.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Implement at method for tuples
4 participants