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

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

Closed
benlesh opened this issue Mar 5, 2024 · 15 comments
Closed

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

benlesh opened this issue Mar 5, 2024 · 15 comments

Comments

@benlesh
Copy link
Collaborator

benlesh commented Mar 5, 2024

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
@bakkot
Copy link
Contributor

bakkot commented Mar 5, 2024

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.

@petamoriken
Copy link

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

@ljharb
Copy link

ljharb commented Aug 17, 2024

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.

@flensrocker
Copy link
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)

@domfarolino
Copy link
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.

@petamoriken
Copy link

@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

@petamoriken
Copy link

petamoriken commented Dec 1, 2024

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

@bakkot
Copy link
Contributor

bakkot commented Dec 1, 2024

@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.)

@petamoriken
Copy link

petamoriken commented Dec 1, 2024

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?

@bakkot
Copy link
Contributor

bakkot commented Dec 1, 2024

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

@petamoriken
Copy link

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

@lucacasonato
Copy link

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

@bakkot
Copy link
Contributor

bakkot commented Dec 1, 2024

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.

@ljharb
Copy link

ljharb commented Dec 1, 2024

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.

@lucacasonato
Copy link

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.

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

No branches or pull requests

7 participants