Skip to content

Observable.from should throw primitive iterables (strings) #125

Description

@benlesh

Along with how Iterator helpers behave, if a user passes a string, even though it's an iterable, it's likely an error. We should throw a TypeError in that case.

Observable.from('some string'); // TypeError

Activity

  1. bakkot commented on Mar 5, 2024

    @bakkot
    Contributor

    I think you misread my comment (which was not all that clear, sorry). Iterator.from does not throw when given a string. This makes it different from flatMap, which does throw when the mapper returns a string, even though strings are iterable.

    See discussion in tc39/proposal-iterator-helpers#244.

  2. petamoriken commented on Aug 17, 2024

    @petamoriken

    FYI: ReadableStream.from rejects string whatwg/webidl#1397

  3. ljharb commented on Aug 17, 2024

    @ljharb

    If someone is intentionally passing a string into an X.from method, they explicitly want to convert a string into an X. I think this is the one case where an iterable string must be accepted.

  4. flensrocker commented on Nov 8, 2024

    @flensrocker
    Contributor

    Can a usecase be returning a string from the catch operator?
    How should I do this if Observable.from throws on a string? Or can I just return the string?
    (having rxjs of("...") in mind)

  5. domfarolino commented on Nov 19, 2024

    @domfarolino
    Collaborator

    I'm sold on ensuring Observable.from("string") works the same as Observable.from(Object("string")) does today, by basically adding special handling like https://github.com/tc39/proposal-iterator-helpers/pull/250/files did for Iterator helpers. I'll ensure that this makes it into #160, and I'll close this issue.

  6. petamoriken commented on Dec 1, 2024

    @petamoriken

    @domfarolino

    I'm sold on ensuring Observable.from("string") works the same as Observable.from(Object("string")) does today, by basically adding special handling like https://github.com/tc39/proposal-iterator-helpers/pull/250/files did for Iterator helpers.

    I think the behavior of Observable.from(“string”) and Observable.from(Object(“string”)) should be clearly separated.

    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.

    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

  7. petamoriken commented on Dec 1, 2024

    @petamoriken

    If you do so, please specify in the specification that primitive strings are accepted in exceptional cases.

  8. bakkot commented on Dec 1, 2024

    @bakkot
    Contributor

    @petamoriken The normative-conventions wording shouldn't apply to explicit coercion methods like Iterator.from and Observable.from. (@michaelficarra, who wrote that wording, was aware of this at one point, but he keeps forgetting.)

  9. petamoriken commented on Dec 1, 2024

    @petamoriken

    Well, I don't understand why ReadableStream.from doesn't accept primitive strings while Observable.from does. Does that mean Observable.from should be treated as primitive as Iterator.from?

  10. bakkot commented on Dec 1, 2024

    @bakkot
    Contributor

    If I understand correctly the current plan is for ReadableStream.from to accept primitive strings. cc @lucacasonato

  11. petamoriken commented on Dec 1, 2024

    @petamoriken

    It does not seem so.
    FYI: denoland/deno#26882, denoland/deno#25116

  12. lucacasonato commented on Dec 1, 2024

    @lucacasonato

    @bakkot No, the plan is to accept boxed strings, but not primitive strings

  13. bakkot commented on Dec 1, 2024

    @bakkot
    Contributor

    Ah. Well, that's unfortunate. In that case I have no opinion about what Observable.from should do, since it can't be consistent with both.

  14. ljharb commented on Dec 1, 2024

    @ljharb

    The entire point of a "from" method is to explicitly convert from one type to another. It's the one place rejecting strings makes no sense (or rephrased, the one place accepting iterable strings makes sense), and if Readable.from wants to be the weird one, that doesn't mean Observable.from should join it.

  15. lucacasonato commented on Dec 1, 2024

    @lucacasonato

    Actually what I said may be wrong. I have to read thru the discussion again. Specifically the async iterable webidl type will not directly accept string primitives. However we might have decided something else for ReadableStream.from. I will have to check again.

  16. benlesh commented on Feb 7, 2025

    @benlesh
    CollaboratorAuthor

    @bakkot and @ljharb thank you for the insight. This makes total sense to me. I'm just used to seeing operators like flatMap implement in terms of Observable.from(fn(value, index++)) or whatever, and in those cases, in RxJS at least, it's almost always a bug when someone returns a string from a switchMap, flatMap/concatMap, mergeMap etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions