Skip to content

Conversation

WeiWen1999
Copy link

closed #369

@WeiWen1999 WeiWen1999 closed this Apr 13, 2025
@WeiWen1999 WeiWen1999 reopened this Apr 13, 2025
@WeiWen1999 WeiWen1999 closed this Apr 13, 2025
@WeiWen1999 WeiWen1999 reopened this Apr 13, 2025
@WeiWen1999 WeiWen1999 force-pushed the feat/array-at branch 2 times, most recently from 21914b3 to 4713e0c Compare May 7, 2025 07:18
@sindresorhus sindresorhus force-pushed the main branch 2 times, most recently from eabb987 to cc2b0f2 Compare May 8, 2025 08:21
@sindresorhus
Copy link
Owner

@giao66 Would you be able to fix the merge conflicts?

@Emiyaaaaa
Copy link
Collaborator

Maybe you can refer this PR: #1085

@WeiWen1999 WeiWen1999 force-pushed the feat/array-at branch 2 times, most recently from fa9905a to 3748962 Compare August 24, 2025 03:33
@sindresorhus
Copy link
Owner

There are still unhandled feedback.

Resolve the comments when you are sure they are resolved.

@WeiWen1999
Copy link
Author

There are still unhandled feedback.

Resolve the comments when you are sure they are resolved.

HELLO! I DONE

@WeiWen1999 WeiWen1999 force-pushed the feat/array-at branch 3 times, most recently from ccb61b3 to 820d69e Compare August 28, 2025 11:51
@sindresorhus
Copy link
Owner

You force-pushed over my changes.

@WeiWen1999
Copy link
Author

You force-pushed over my changes.

So I should sync the main branch first before pushing, right?

@WeiWen1999
Copy link
Author

You force-pushed over my changes.

Sorry,This is my first time trying to contribute to an open-source project.

@WeiWen1999
Copy link
Author

You force-pushed over my changes.

Can this version be restored?
p

@sindresorhus
Copy link
Owner

It's fine. I will do the changes again after merging.

@sindresorhus
Copy link
Owner

AI review:


  1. Stray debug/types in the public file

    • source/array-at.d.ts contains scratch types like type TrailingSpreadArray2, type MiddleSpreadArray, type A/B/C, etc. These should never ship in a public .d.ts. Remove them. ([GitHub]1)
  2. Dead import(s)

    • GreaterThan is imported but not used (I only see GreaterThanOrEqual/LessThan/LessThanOrEqual being referenced). Drop it. Keep imports minimal to reduce checker work. ([GitHub]1)
  3. Unknown index handling (generic N)

    • Top level signature allows N extends number. If N is not a literal, comparisons like IsNegative<N>/LessThan<N, …> become indeterminate and you get brittle distributive behavior or over-narrowing.

    • Add a fast path: for unknown N, return a safe union.

      • For fixed-length tuples: number extends N ? (IsExactOptionalPropertyTypesEnabled extends true ? Required<T>[number] : T[number]) | undefined : …
      • For non-fixed arrays: number extends N ? T[number] : …
    • This mirrors runtime: unknown index => “some element (maybe) or undefined”. It also prevents the type from collapsing to never in some branches.

Potential logic gaps to double-check

  • Fixed tuple, out-of-bounds negative: The Sum<T['length'], N> branch correctly yields undefined when the computed index is still negative. Keep that, but cover it with tsd (e.g., ArrayAt<[1, 2, 3], -4> -> undefined). ([GitHub]1)
  • Trailing spread, negative indices: The unioning via the “slice then …infer Last” approach looks right (e.g., [A, B, ...C[]], -1 => B | C | undefined depending on spread arity), but you must assert it with tests—especially for -2, -3, etc. where unions widen quickly. ([GitHub]1)
  • Leading spread, positive indices: The positive branch unions the possible holders for that position: ArraySlice<StaticPart, 0, N+1>[number] | VariablePart[number] | (N≥len ? undefined : never). This is conceptually correct; again, codify with tests. ([GitHub]1)
  • Middle spread recursion: The recursion to NonFixedLengthArrayAt<[...VariablePart, ...TrailingStaticPart], …> (for positive) and the symmetric negative handling looking into TrailingStaticPart is good, but performance-sensitive. Make sure tsd stays fast on large tuples (see perf note below). ([GitHub]1)

Tests you’re missing (add to test-d/array-at.ts)

Cover each of these explicitly with tsd assertions:

Fixed-length tuples

  • ArrayAt<[], 0> -> undefined

  • ArrayAt<[A], 0> -> A

  • ArrayAt<[A], -1> -> A

  • ArrayAt<[A, B, C], -1> -> C

  • ArrayAt<[A, B, C], -4> -> undefined

  • Optional elements with and without exactOptionalPropertyTypes:

    • ArrayAt<[A?, B], 0> under both modes
    • ArrayAt<[A?, B?], -1> under both modes

Non-fixed arrays

  • ArrayAt<string[], 0> -> string
  • ArrayAt<string[], -1> -> string | undefined (document the choice you want; if you keep it as just string, state why)
  • ArrayAt<readonly number[], 1> -> number (readonly preservation)

Spread patterns

  • Leading: ArrayAt<[...string[], A, B], 0 | 1 | 2 | -1>
  • Middle: ArrayAt<[A, ...string[], B], 0 | 1 | 2 | -1 | -2>
  • Trailing: ArrayAt<[A, B, ...string[]], -1 | -2 | -3>
  • Mixed optional statics like [A?, ...string[], B?] near boundaries

Unknown index

  • ArrayAt<[A, B, C], number> – should be A | B | C | undefined (or at least A | B | C; pick and document)
  • ArrayAt<string[], number>string

Cleanups / polish

  • Remove the stray scratch types from source/array-at.d.ts. ([GitHub]1)
  • Remove unused GreaterThan import. ([GitHub]1)
  • Consider extracting tiny helpers to reduce repetition (NormalizeIndex<T,N>, etc.) if it doesn’t regress checker perf.

Docs

  • README entry is fine, but add one more example showing a negative index on a tuple and on a trailing-spread case, plus a note that for non-fixed arrays the result is a union (and may or may not include undefined, per your chosen semantics). ([GitHub]1)

Perf

  • This kind of type explodes unions quickly. Run the usual tsc --extendedDiagnostics locally on the repo’s test suite once you add the broader cases. If anything spikes, prefer fewer nested extends infer steps in the hot branches.

TL;DR required changes before merge

  • Delete the debug types from source/array-at.d.ts. Blocker. ([GitHub]1)
  • Drop unused GreaterThan import. Blocker. ([GitHub]1)
  • Add an explicit unknown-index guard for N extends number (see above). Strongly recommended.
  • Add the missing tsd cases outlined above, especially around exactOptionalPropertyTypes. Strongly recommended. ([TypeScript]2)

@sindresorhus
Copy link
Owner

After doing the above, I recommend asking ChatGPT to review your pull request and ask it to add more tests if needed.

@WeiWen1999
Copy link
Author

After doing the above, I recommend asking ChatGPT to review your pull request and ask it to add more tests if needed.

ok,wizard.Thanks a lot!

@sindresorhus
Copy link
Owner

  • Wrong Every docs (copy-paste). The block under export type Every… documents CollapseRestElement with unrelated examples. Replace with correct description and examples for Every<TArray, Type>. ([GitHub][1])

  • Misuse of IsEqual (no type args). In StaticPartOfArray, you wrote IsEqual extends true without parameters, which is invalid. Use an actual comparison, e.g., compare the head optional element:

    type HeadOptional =
    	OptionalPartOfStaticArray<T> extends [infer O] ? O : never;
    
    type HasOptionalHead = IsEqual<HeadOptional, undefined>;
    // …
    : HasOptionalHead extends true ? Result : [...Result, Required<T>[0]?]

(Adjust to your existing generics for T, Result.) ([GitHub][1])

  • Duplicate index computation (leading-spread, negative index). In NonFixedLengthArrayAt’s leading-spread negative branch you compute Sum<StaticPart['length'], N> extends infer Index twice. Compute once and reuse. ([GitHub][1])

  • Doc wording mismatch. LeadingStaticPartOfMiddleSpreadArray comment says “leading variable, non-fixed-length portion,” but the name is “StaticPart.” Change “variable” → “static” to avoid confusion. ([GitHub][1])

  • Missing internal re-exports. source/array-at.d.ts imports many helpers from ./internal/index.d.ts, but this PR doesn’t add those exports to that aggregator. Add them or import directly from ./internal/array.d.ts to prevent broken imports. ([GitHub][1])

  • Examples in ArrayAt JSDoc are wrong/incomplete. They show type B = ArrayAt; without type parameters. Use concrete, compilable examples (both positive and negative indices):

    type B = ArrayAt<[string, number, boolean], 0>;  // string
    type C = ArrayAt<[string, number, boolean], -1>; // boolean
    type D = ArrayAt<string[], -1>;                  // string | undefined
    ``` :contentReference[oaicite:5]{index=5}
  • Over-defensive conditional in fixed-tuple branch. Index extends unknown ? … : never is redundant (always true). Drop the extra layer for readability.

  • Test coverage gaps. Add:

    • readonly tuples.
    • Union index literals (0 | 2, -1 | -3).
    • Union of array kinds (string[] | number[]).
    • Negative out-of-bounds on empty tuple (ArrayAt<[], -1>).
      Current tests cover many shapes but miss these. ([GitHub][1])
  • Minor housekeeping. Keep the index.d.ts export grouped with other array utilities (ArraySlice, ArraySplice) for discoverability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ArrayAt type for stricter Array.prototype.at() with tuples
4 participants