Preserve `Array.modify` and `Array.modifyOption` non emptiness #3496

vinassefranche · 2024-08-22T08:32:01Z

Type

Description

Same as #3491 but for modify functions

Name	Type
effect	Minor
@effect/cli	Major
@effect/cluster-browser	Major
@effect/cluster-node	Major
@effect/cluster-workflow	Major
@effect/cluster	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-d1	Major
@effect/sql-drizzle	Major
@effect/sql-kysely	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

vinassefranche · 2024-08-22T09:36:01Z

I'm looking at the doc generation issue. There seems to be a real issue with the way the parameter function passed to Array.modify is now inferred.

vinassefranche · 2024-08-22T09:46:03Z

packages/effect/src/Array.ts

@@ -1097,6 1097,8 @@ export const replaceOption: {
 <A, B>(self: Iterable<A>, i: number, b: B): Option<Array<A | B>> => modifyOption(self, i, () => b)
 )

+type TypeOfIterable<T extends Iterable<any>> = T extends Iterable<infer A> ? A : never


This is how I fixed the inference of a. Tell me if:

it's a valid way to fix the issue

it's the correct place to put this kind of the type helpers (or if it already exists somewhere. I did not find anything on my own)

gcanti · 2024-08-22T10:21:21Z

TypeOfIterable

you can use ReadonlyArray.Infer<S> (also I think this change needs more dtslint tests)

i.e.

/**
 * Apply a function to the element at the specified index, creating a new `Array`,
 * or return a copy of the input if the index is out of bounds.
 *
 * @example
 * import { Array } from "effect"
 *
 * const numbers = [1, 2, 3, 4]
 * const result = Array.modify(numbers, 2, (n) => n * 2)
 * assert.deepStrictEqual(result, [1, 2, 6, 4])
 *
 * @since 2.0.0
 */
export const modify: {
  <A, B, S extends Iterable<A> = Iterable<A>>(
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): (self: S) => ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>
  <A, B, S extends Iterable<A> = Iterable<A>>(
    self: S,
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>
} = dual(
  3,
  <A, B>(self: Iterable<A>, i: number, f: (a: A) => B): Array<A | B> =>
    O.getOrElse(modifyOption(self, i, f), () => Array.from(self))
)

/**
 * Apply a function to the element at the specified index, creating a new `Array`,
 * or return `None` if the index is out of bounds.
 *
 * @example
 * import { Array, Option } from "effect"
 *
 * const numbers = [1, 2, 3, 4]
 * const result = Array.modifyOption(numbers, 2, (n) => n * 2)
 * assert.deepStrictEqual(result, Option.some([1, 2, 6, 4]))
 *
 * const outOfBoundsResult = Array.modifyOption(numbers, 5, (n) => n * 2)
 * assert.deepStrictEqual(outOfBoundsResult, Option.none())
 *
 * @since 2.0.0
 */
export const modifyOption: {
  <A, B, S extends Iterable<A> = Iterable<A>>(
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): (self: S) => Option<ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>>
  <A, B, S extends Iterable<A> = Iterable<A>>(
    self: S,
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): Option<ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>>
} = dual(3, <A, B>(self: Iterable<A>, i: number, f: (a: A) => B): Option<Array<A | B>> => {
  const out = Array.from(self)
  if (isOutOfBound(i, out)) {
    return O.none()
  }
  const next = f(out[i])
  // @ts-expect-error
  out[i] = next
  return O.some(out)
})

tested against

// -------------------------------------------------------------------------------------
// modify
// -------------------------------------------------------------------------------------

// $ExpectType string[]
Array.modify([], 0, (
  _n // $ExpectType never
) => "a")

// $ExpectType (string | number)[]
Array.modify(numbers, 0, (
  _n // $ExpectType number
) => "a")

// $ExpectType [number | "a", ...(number | "a")[]]
Array.modify(nonEmptyNumbers, 0, (
  _n // $ExpectType number
) => "a" as const)

// $ExpectType ("a" | 1 | 2)[]
Array.modify(new Set([1, 2] as const), 0, (
  _n // $ExpectType 1 | 2
) => "a" as const)

// $ExpectType string[]
pipe(
  [],
  Array.modify(0, (
    _n // $ExpectType never
  ) => "a")
)

// $ExpectType (string | number)[]
pipe(
  numbers,
  Array.modify(0, (
    _n // $ExpectType number
  ) => "a")
)

// $ExpectType [number | "a", ...(number | "a")[]]
pipe(
  nonEmptyNumbers,
  Array.modify(0, (
    _n // $ExpectType number
  ) => "a" as const)
)

// $ExpectType ("a" | 1 | 2)[]
pipe(
  new Set([1, 2] as const),
  Array.modify(0, (
    _n // $ExpectType 1 | 2
  ) => "a" as const)
)

// -------------------------------------------------------------------------------------
// modifyOption
// -------------------------------------------------------------------------------------

// $ExpectType Option<string[]>
Array.modifyOption([], 0, (
  _n // $ExpectType never
) => "a")

// $ExpectType Option<(string | number)[]>
Array.modifyOption(numbers, 0, (
  _n // $ExpectType number
) => "a")

// $ExpectType Option<[number | "a", ...(number | "a")[]]>
Array.modifyOption(nonEmptyNumbers, 0, (
  _n // $ExpectType number
) => "a" as const)

// $ExpectType Option<("a" | 1 | 2)[]>
Array.modifyOption(new Set([1, 2] as const), 0, (
  _n // $ExpectType 1 | 2
) => "a" as const)

// $ExpectType Option<string[]>
pipe(
  [],
  Array.modifyOption(0, (
    _n // $ExpectType never
  ) => "a")
)

// $ExpectType Option<(string | number)[]>
pipe(
  numbers,
  Array.modifyOption(0, (
    _n // $ExpectType number
  ) => "a")
)

// $ExpectType Option<[number | "a", ...(number | "a")[]]>
pipe(
  nonEmptyNumbers,
  Array.modifyOption(0, (
    _n // $ExpectType number
  ) => "a" as const)
)

// $ExpectType Option<("a" | 1 | 2)[]>
pipe(
  new Set([1, 2] as const),
  Array.modifyOption(0, (
    _n // $ExpectType 1 | 2
  ) => "a" as const)
)

vinassefranche · 2024-08-22T10:26:54Z

TypeOfIterable

you can use ReadonlyArray.Infer<S> (also I think this change needs more dtslint tests)

i.e.

/**
 * Apply a function to the element at the specified index, creating a new `Array`,
 * or return a copy of the input if the index is out of bounds.
 *
 * @example
 * import { Array } from "effect"
 *
 * const numbers = [1, 2, 3, 4]
 * const result = Array.modify(numbers, 2, (n) => n * 2)
 * assert.deepStrictEqual(result, [1, 2, 6, 4])
 *
 * @since 2.0.0
 */
export const modify: {
  <A, B, S extends Iterable<A> = Iterable<A>>(
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): (self: S) => ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>
  <A, B, S extends Iterable<A> = Iterable<A>>(
    self: S,
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>
} = dual(
  3,
  <A, B>(self: Iterable<A>, i: number, f: (a: A) => B): Array<A | B> =>
    O.getOrElse(modifyOption(self, i, f), () => Array.from(self))
)

/**
 * Apply a function to the element at the specified index, creating a new `Array`,
 * or return `None` if the index is out of bounds.
 *
 * @example
 * import { Array, Option } from "effect"
 *
 * const numbers = [1, 2, 3, 4]
 * const result = Array.modifyOption(numbers, 2, (n) => n * 2)
 * assert.deepStrictEqual(result, Option.some([1, 2, 6, 4]))
 *
 * const outOfBoundsResult = Array.modifyOption(numbers, 5, (n) => n * 2)
 * assert.deepStrictEqual(outOfBoundsResult, Option.none())
 *
 * @since 2.0.0
 */
export const modifyOption: {
  <A, B, S extends Iterable<A> = Iterable<A>>(
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): (self: S) => Option<ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>>
  <A, B, S extends Iterable<A> = Iterable<A>>(
    self: S,
    i: number,
    f: (a: ReadonlyArray.Infer<S>) => B
  ): Option<ReadonlyArray.With<S, ReadonlyArray.Infer<S> | B>>
} = dual(3, <A, B>(self: Iterable<A>, i: number, f: (a: A) => B): Option<Array<A | B>> => {
  const out = Array.from(self)
  if (isOutOfBound(i, out)) {
    return O.none()
  }
  const next = f(out[i])
  // @ts-expect-error
  out[i] = next
  return O.some(out)
})

tested against

// -------------------------------------------------------------------------------------
// modify
// -------------------------------------------------------------------------------------

// $ExpectType string[]
Array.modify([], 0, (
  _n // $ExpectType never
) => "a")

// $ExpectType (string | number)[]
Array.modify(numbers, 0, (
  _n // $ExpectType number
) => "a")

// $ExpectType [number | "a", ...(number | "a")[]]
Array.modify(nonEmptyNumbers, 0, (
  _n // $ExpectType number
) => "a" as const)

// $ExpectType ("a" | 1 | 2)[]
Array.modify(new Set([1, 2] as const), 0, (
  _n // $ExpectType 1 | 2
) => "a" as const)

// $ExpectType string[]
pipe(
  [],
  Array.modify(0, (
    _n // $ExpectType never
  ) => "a")
)

// $ExpectType (string | number)[]
pipe(
  numbers,
  Array.modify(0, (
    _n // $ExpectType number
  ) => "a")
)

// $ExpectType [number | "a", ...(number | "a")[]]
pipe(
  nonEmptyNumbers,
  Array.modify(0, (
    _n // $ExpectType number
  ) => "a" as const)
)

// $ExpectType ("a" | 1 | 2)[]
pipe(
  new Set([1, 2] as const),
  Array.modify(0, (
    _n // $ExpectType 1 | 2
  ) => "a" as const)
)

// -------------------------------------------------------------------------------------
// modifyOption
// -------------------------------------------------------------------------------------

// $ExpectType Option<string[]>
Array.modifyOption([], 0, (
  _n // $ExpectType never
) => "a")

// $ExpectType Option<(string | number)[]>
Array.modifyOption(numbers, 0, (
  _n // $ExpectType number
) => "a")

// $ExpectType Option<[number | "a", ...(number | "a")[]]>
Array.modifyOption(nonEmptyNumbers, 0, (
  _n // $ExpectType number
) => "a" as const)

// $ExpectType Option<("a" | 1 | 2)[]>
Array.modifyOption(new Set([1, 2] as const), 0, (
  _n // $ExpectType 1 | 2
) => "a" as const)

// $ExpectType Option<string[]>
pipe(
  [],
  Array.modifyOption(0, (
    _n // $ExpectType never
  ) => "a")
)

// $ExpectType Option<(string | number)[]>
pipe(
  numbers,
  Array.modifyOption(0, (
    _n // $ExpectType number
  ) => "a")
)

// $ExpectType Option<[number | "a", ...(number | "a")[]]>
pipe(
  nonEmptyNumbers,
  Array.modifyOption(0, (
    _n // $ExpectType number
  ) => "a" as const)
)

// $ExpectType Option<("a" | 1 | 2)[]>
pipe(
  new Set([1, 2] as const),
  Array.modifyOption(0, (
    _n // $ExpectType 1 | 2
  ) => "a" as const)
)

I did the changes. Thanks for the review!

gcanti · 2024-08-22T10:40:58Z

I did the changes

not all of them actually, take a closer look at the source code here #3496 (comment)
(namely the data-last variants)

vinassefranche · 2024-08-22T11:45:57Z

I did the changes

not all of them actually, take a closer look at the source code here #3496 (comment) (namely the data-last variants)

@gcanti Why do you think it's needed? As S is only used in the "second" function, there's no need to define the type parameter upfront.
It seems to be working fine like this. Did you find a case where something is wrong?

vinassefranche · 2024-08-22T11:52:46Z

I did the changes

not all of them actually, take a closer look at the source code here #3496 (comment) (namely the data-last variants)

@gcanti Why do you think it's needed? As S is only used in the "second" function, there's no need to define the type parameter upfront. It seems to be working fine like this. Did you find a case where something is wrong?

@gcanti My bad, I did not run the dtslint job. It's annoying that it's not visible in Vscode.

mikearnaldi · 2024-08-22T11:53:52Z

It is if you install the eslint plugin

vinassefranche · 2024-08-22T11:58:09Z

It is if you install the eslint plugin

Eslint plugin? You're talking about an extension or a real eslint plugin
I have the official eslint extension installed on vscode but I don't see the dtslint issues in the file. See this screenshot:

Am I missing something or are we not discussing about the same thing?

mikearnaldi · 2024-08-22T11:59:28Z

Ah no apologies I thought you were referring to an eslint error, no unfortunately dtslint is separate

…t-TS#3496)

Preserve non emptiness for Array.modify and Array.modifyOption

3d271ba

vinassefranche requested a review from mikearnaldi as a code owner August 22, 2024 08:32

Add changeset

2151140

Fix bad inference of function parameter

b58cdd4

vinassefranche commented Aug 22, 2024

View reviewed changes

vinassefranche added 2 commits August 22, 2024 12:20

Merge branch 'next-minor' into preserve-array-replace-non-emptiness

51c716a

Separate modify and modifyOption in dtslint

a47f382

vinassefranche added 2 commits August 22, 2024 12:24

Use existing type helper

dc32dce

Improve dtslint tests

ec4acb7

Fix inference of n parameter for data last variant

da5813e

gcanti merged commit ca5320c into Effect-TS:next-minor Aug 22, 2024
11 checks passed

github-actions bot mentioned this pull request Aug 22, 2024

Release queue: minor #3410

Merged

vinassefranche deleted the preserve-array-replace-non-emptiness branch August 22, 2024 12:40

dilame pushed a commit to dilame/effect that referenced this pull request Aug 22, 2024

Preserve Array.modify and Array.modifyOption non emptiness (Effec…

0229c2d

…t-TS#3496)

github-actions bot pushed a commit that referenced this pull request Aug 23, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

6fcf117

github-actions bot pushed a commit that referenced this pull request Aug 23, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

38ec163

github-actions bot pushed a commit that referenced this pull request Aug 26, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

5ea0c51

github-actions bot pushed a commit that referenced this pull request Aug 27, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

f2d8ec7

github-actions bot pushed a commit that referenced this pull request Aug 27, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

601a657

github-actions bot pushed a commit that referenced this pull request Aug 27, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

10d3699

github-actions bot pushed a commit that referenced this pull request Aug 29, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

2973d52

github-actions bot pushed a commit that referenced this pull request Aug 29, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

aaea134

github-actions bot pushed a commit that referenced this pull request Aug 30, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

ab3e1cf

tim-smart pushed a commit that referenced this pull request Aug 30, 2024

Preserve Array.modify and Array.modifyOption non emptiness (#3496)

2f456cc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve `Array.modify` and `Array.modifyOption` non emptiness #3496

Preserve `Array.modify` and `Array.modifyOption` non emptiness #3496

vinassefranche commented Aug 22, 2024

changeset-bot bot commented Aug 22, 2024 •

edited

Loading

vinassefranche commented Aug 22, 2024

vinassefranche Aug 22, 2024

gcanti commented Aug 22, 2024

vinassefranche commented Aug 22, 2024

gcanti commented Aug 22, 2024 •

edited

Loading

vinassefranche commented Aug 22, 2024

vinassefranche commented Aug 22, 2024

mikearnaldi commented Aug 22, 2024

vinassefranche commented Aug 22, 2024

mikearnaldi commented Aug 22, 2024

Preserve Array.modify and Array.modifyOption non emptiness #3496

Preserve Array.modify and Array.modifyOption non emptiness #3496

Conversation

vinassefranche commented Aug 22, 2024

Type

Description

Related

changeset-bot bot commented Aug 22, 2024 • edited Loading

🦋 Changeset detected

vinassefranche commented Aug 22, 2024

vinassefranche Aug 22, 2024

Choose a reason for hiding this comment

gcanti commented Aug 22, 2024

vinassefranche commented Aug 22, 2024

gcanti commented Aug 22, 2024 • edited Loading

vinassefranche commented Aug 22, 2024

vinassefranche commented Aug 22, 2024

mikearnaldi commented Aug 22, 2024

vinassefranche commented Aug 22, 2024

mikearnaldi commented Aug 22, 2024

Preserve `Array.modify` and `Array.modifyOption` non emptiness #3496

Preserve `Array.modify` and `Array.modifyOption` non emptiness #3496

changeset-bot bot commented Aug 22, 2024 •

edited

Loading

gcanti commented Aug 22, 2024 •

edited

Loading