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

Add --strictObjectIterables (bikeshed): exclude primitive (string) from Iterable types #59676

Open
6 tasks done
petamoriken opened this issue Aug 18, 2024 · 3 comments
Open
6 tasks done
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@petamoriken
Copy link
Contributor

πŸ” Search Terms

"iterable object", "string"

βœ… Viability Checklist

⭐ Suggestion

At the TC39 meeting in 2024-07, it was decided that Iterable expects objects.

Reject primitives in iterable-taking positions

Any time an iterable or async-iterable value (a value that has a Symbol.iterator or Symbol.asyncIterator method) is expected, primitives should be treated as if they were not iterable. Usually, this will mean throwing a TypeError. If the user provides a primitive wrapper Object such as a String Object, however, it should be treated like any other Object.

Although primitive Strings are default iterable (String.prototype has a Symbol.iterator method which enumerates code points), it is now considered a mistake to iterate a String without specifying whether the String is providing an abstraction over code units, code points, grapheme clusters, or something else.

NB: This convention is new as of 2024, and most earlier parts of the language do not follow it. In particular, positional destructuring (both binding and assignment), array spread, argument spread, for-of loops, yield *, the Set and AggregateError constructors, Object.groupBy, Map.groupBy, Promise.all, Promise.allSettled, Promise.any, Promise.race, Array.from, the static from methods on typed array constructors, and Iterator.from (Stage 3 at time of writing) all accept primitives where iterables are expected.

https://github.com/tc39/how-we-work/blob/main/normative-conventions.md#reject-primitives-in-iterable-taking-positions

To follow this decision, the Iterable and string types should be separated, and APIs that accept both types should be required to explicitly specify Iterable<any> | string. Since this would be a breaking change, how about adding a new --strictObjectIterables (bikeshed) option?

In practice, the upcoming ReadableStream.from will accept Iterable<any> and AsyncIterable<any>, but will be restricted to objects. whatwg/streams#1310

πŸ“ƒ Motivating Example

Enabling the --strictObjectIterables option raises a type error in the following example:

const iterable: Iterable<any> = "string";

πŸ’» Use Cases

  1. What do you want to use this for?

    Used for ReadableStream.from and other APIs to be added in the future.

  2. What shortcomings exist with current approaches?

    Iterable object and string are not distinguished by default.

  3. What workarounds are you using in the meantime?

    Maybe Iterable<any> & object can reject string.

@nmain
Copy link

nmain commented Aug 19, 2024

"Old" functions such as Array.from will still accept primitive iterables at runtime, and still do something meaningful with them. IMO, those should be typed differently than any new function that rejects primitive iterables.

@RyanCavanaugh
Copy link
Member

The results at #59683 outline the difficulty of this. Looking at the code, we see a lot of places where people write code that takes an implicit dependency on the iterability of strings, e.g. this kind of code is idiomatic JS and looks to be 100% correct:

// https://github.com/refined-github/refined-github/blob/6e167b9d881bd05db8cb92a5e140144cda7fd873/source/features/quick-review.tsx#L18C1-L18C54
const emojis = [...'πŸš€πŸΏοΈβš‘οΈπŸ€ŒπŸ₯³πŸ₯°πŸ€©πŸ₯ΈπŸ˜ŽπŸ€―πŸš’πŸ›«πŸ³οΈπŸ'];

There's just a ton of code out there that takes inputs like URLs, hashes, file paths, programmer-authored strings, etc, and iterates it in a way that can't cause problems.

We also see places where people forget that strings are iterable and end up passing e.g. "foo" into a context where it will be interpreted as "f", "o", "o", which is a clear error:

// https://github.com/mui/material-ui/blob/8ab2137281267ad4fa766e4bd64d03baa5d44820/docs/data/material/getting-started/templates/dashboard/theme/themePrimitives.ts#L227
  shadows: [
    'none',
    ...(mode === 'dark'
      ? 'hsla(220, 30%, 5%, 0.7) 0px 4px 16px 0px, hsla(220, 25%, 10%, 0.8) 0px 8px 16px -5px'
      : 'hsla(220, 30%, 5%, 0.07) 0px 4px 16px 0px, hsla(220, 25%, 10%, 0.07) 0px 8px 16px -5px'),
  ] as Shadows

But both of these are just the same thing: spreading a string. The same thing happened for implicit Symbol.iterable calls. There doesn't seem to be a good way to tell, as a machine, if someone is iterating a string on purpose or not.

Worse, the iterator capability of string is the only way to do certain "correct" Unicode operations, e.g.:

console.log("πŸ‘ͺ".length); // 2, probably expected 1
console.log([..."πŸ‘ͺ"].length); // single codepoint, ok-ish

and we did see code that intentionally uses this behavior.

We'd need some way to tell apart intentional from non-intentional iteration to proceed here, and one doesn't seem to exist in the type system. Open to ideas, though.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Aug 19, 2024
@MichaelMitchell-at
Copy link

Worse, the iterator capability of string is the only way to do certain "correct" Unicode operations, e.g.:

Similar to what @nmain mentioned above perhaps Array.from could be given an overload to explicitly accept a string as an argument and it becomes the only blessed way to do things like Array.from("πŸ‘ͺ") rather than [..."πŸ‘ͺ"].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants