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

docs request: help me figure out what to do with JS unions when they occur #337

Open
csells opened this issue Jan 19, 2025 · 4 comments
Open
Labels
type-enhancement A request for a change that isn't a bug

Comments

@csells
Copy link

csells commented Jan 19, 2025

As an example, the docs for ReadableStream.getReader() says that it returns a ReadableStreamReader. Cool.

Except the docs for ReadableStreamReader says that it's just a JSObject.

Now what am I supposed to do?

After talking to @kevmoo, I learned that this happens when there's a JS union, which Dart doesn't support. Fair enough. I also learned that the MDN docs for getReader() tell me that the possible JS returns are ReadableStreamDefaultReader or ReadableStreamBYOBReader, both of which have Dart equivalents. However neither of these types is mentioned in the Dart web getReader() docs or the ReadableStreamReader docs, so how do I know that if I don't talk to Kevin?

With this info, my first thought is to do a Dart cast to ReadableStreamDefaultReader (the default) but according to Kevin, I'm not supposed to do that either. Apparently there's some lint that tells me what to do. Which is great, if I've got that lint enabled.

But even if I do have that lint enabled, the docs should tell me not to waste my time on Dart as or is to learn from the lint (if I have it enabled) what the right thing to do is.

The MDN docs tell me what the right thing to do is. And it's my understanding that the Dart docs are generated from the MDN docs (or the same metadata that the MDN docs are generated from). So can we please make the Dart docs tell me what the right thing to do in the face of a JS union is?

Thank you. And thank you for attending my Ted talk.

@srujzs srujzs added the type-enhancement A request for a change that isn't a bug label Jan 22, 2025
@srujzs
Copy link
Contributor

srujzs commented Jan 22, 2025

I like that idea! A simple doc comment illustrating that this is a typedef that represents a union of [ReadableStreamBYOBReader] and [ReadableStreamDefaultReader] would tell users what types to type-check against if they need to. This would be generated directly from the Web IDL though, not the MDN.

There's a general section on type-checking in the docs with JS interop, but we can always improve it: https://dart.dev/interop/js-interop/js-types#compatibility-type-checks-and-casts. Maybe we can add a comment saying to use isA on top of every union, but maybe that's too verbose. Maybe you don't even need to use isA because you already know what the intended type is. At the minimum, we should say what types the union corresponds to, but I can be convinced either way of whether we should tell users what to do with that info on the typedef itself.

@csells
Copy link
Author

csells commented Jan 22, 2025

Thanks, @srujzs. If I don't do the isA, how do I get the type out? My understanding from @kevmoo is that I can't use the standard type mechanism. Either way, I think a short, auto-generated code sample of how to cast to the available union types would be a welcome, not verbose, addition to those docs.

@srujzs
Copy link
Contributor

srujzs commented Jan 22, 2025

Kevin is right that you can't use is, but you can and should downcast a JS value to a JS type using as. I agree that in most cases, users will want to do a type-check before the cast, so I agree that adding what you suggested would be a welcome change.

One thing we need to watch out for when generating these docs is accidentally telling users to use isA with dictionary types. Those types don't have a type besides Object, so you should never use isA with them.

@csells
Copy link
Author

csells commented Jan 22, 2025

Oh my. All the more reason to generate the docs and sample code to let folks know how to access the objects properly. As it is, we only have you and Kevin. : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants