denoland / std

The Deno Standard Library
https://jsr.io/@std
MIT License
3k stars 602 forks source link

Accept readonly array inputs for all functions/methods that don't need to mutate their data #5831

Open lionel-rowe opened 3 weeks ago

lionel-rowe commented 3 weeks ago

Further to https://github.com/denoland/std/issues/5734

Is your feature request related to a problem? Please describe.

Currently, passing data marked as const to various @std functions/methods fails type checking, even though the passed data is never modified by the function in question.

For example, msgpack/encode:

import { encode, type ValueType } from "jsr:@std/msgpack/encode";

const data = {
  a: 1,
  b: { c: 2n },
  d: [3, { e: 4 }],
} as const;

// Argument of type '{ readonly a: 1; readonly b: { readonly c: 2; }; readonly d: readonly [3, { readonly e: 4; }]; }' is not assignable to parameter of type 'ValueType'.
//   Type '{ readonly a: 1; readonly b: { readonly c: 2; }; readonly d: readonly [3, { readonly e: 4; }]; }' is not assignable to type 'ValueMap'.
//     Property 'd' is incompatible with index signature.
//       Type 'readonly [3, { readonly e: 4; }]' is not assignable to type 'ValueType'.
//         Type 'readonly [3, { readonly e: 4; }]' is not assignable to type 'ValueMap'.
//           Index signature for type 'string' is missing in type 'readonly [3, { readonly e: 4; }]'.deno-ts(2345)
encode(data);

Describe the solution you'd like

Mark all array arguments as readonly unless they are mutated by the function in question. The readonly-ness then becomes explicitly part of the contract of the function, hence changing the implementation such that it mutates such arrays would be a breaking change (typically it'd already be a breaking change though, given that consumers typically don't expect functions to mutate their inputs unless such mutation is explicitly the purpose of the function).

Describe alternatives you've considered

Using the same example data as above:

// weirdly still fails type checking
encode(structuredClone(data));

// throws due to bigint
encode(JSON.parse(JSON.stringify(data)));

// passes type checking but loses all type safety and fails `deno-lint(no-explicit-any)`
encode(data as any);

// eww
encode(data as unknown as ValueType);

// passes but verbose/brittle and incurs a runtime performance cost
encode({ ...data, d: [...data.d] });

// probably easy enough to create a `Mutable` utility type that'd work here,
// but shouldn't be necessary given the data is never mutated
encode(data as Mutable<typeof data>);
iuioiua commented 3 weeks ago

This seems like good practice. +1 from me.