Skip to content

Cross-link extensions on class where they apply #2021

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

Closed
Tracked by #39049
munificent opened this issue Sep 11, 2019 · 17 comments
Closed
Tracked by #39049

Cross-link extensions on class where they apply #2021

munificent opened this issue Sep 11, 2019 · 17 comments
Assignees
Labels
customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug

Comments

@munificent
Copy link
Member

Issue #2001 is about documenting extensions themselves at the point they are defined. That's canonical "place" for an extension's docs.

One concern some users have is that users will run into this scenario.

  1. They see some code like foo.bar(). They want to know what bar() is.
  2. They know foo is a Foo, so they look up the API docs for Foo.
  3. No bar().

What they don't realize is that bar() is an extension method defined elsewhere. In the general case, this can't be solved. Users can define extensions on types outside of their own packages, even on core library types or non-class types that have no docs.

But in many cases, the type the extension applies to is a class in the same package where the extension itself is defined. In those cases, it would be nice if the dartdoc for that class could cross-link to the relevant extensions. For example, the C# docs for the IEnumerable interface have a section for extension methods the .NET core library defines on that type.

cc @mit-mit

@munificent munificent added the type-enhancement A request for a change that isn't a bug label Sep 11, 2019
@jcollins-g jcollins-g added customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures labels Sep 30, 2019
@jcollins-g
Copy link
Contributor

Cases like our dart:ffi's Pointer, where extension methods are being used as a primitive sort of method overloading, may require some additional thought as expanding all possible methods on the class page could be unreadable. A followup to #2053 will have a first pass idea of what that might look like.

@parlough
Copy link
Member

parlough commented Oct 11, 2023

As more and more SDK methods are being added as extensions, it's getting more important to flesh out (and implement) a design for this. Including how to handle cases like the dart:ffi Pointer ones that Janice mentioned earlier. This could perhaps be a new section for extension methods, then a dropdown for extension methods with the same name?

DateTime.copyWith is a recent example of being hard to discover (as illustrated by dart-lang/sdk#53729):

I don't think people will think to look at the DateTime extensions, then dive into a specific one to learn more, making it quite impossible to discover.

Edit: I think this would be an interesting issue for someone looking to start contributing to Dart to take on. I reached out to a few people but if anyone is interested in working on this, consider responding here. I'd be happy to help if needed (as much as I can 🙈).

@jakemac53
Copy link
Contributor

Any chance we can circle back to this? The list of applicable extensions as it exists today is easy to miss, and requires opening N different pages to see the extension methods.

The .net example linked above I think is much nicer. I am not sure why from a user perspective why we wouldn't treat extension methods in exactly the same way as regular methods? They could live in a different section to be sure, but it is reasonable to assume extensions from the same package (or maybe, library) are meant to be first class citizens.

@parlough
Copy link
Member

parlough commented Feb 5, 2024

I agree. I think this is super important now with dart:js_interop set up the way it is.

I just think some UX questions remain. For example, Pointer has a bunch of extensions like Int8Pointer on Pointer<Int8> or the equivalent. Each of these has 4-7 or so properties/operators/methods that are often the same name as other extensions.

Cases like these will probably need some form of grouping and collapsing or something.

@srawlins srawlins added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Feb 5, 2024
@srawlins
Copy link
Member

srawlins commented Feb 5, 2024

Yeah let's prioritize this as P1 for the next release.

@srawlins
Copy link
Member

I'm getting my ducks in a row to land this. There's a technical complication with allModelElements. It might be important that a class's allModelElements now include potentially applicable extensions, but doing so would mean that, in order to calculate one class's allModelElements, you must know about all extensions first. This violates a couple properties of the package graph. So I need to sort things out a bit.

@bwilkerson
Copy link
Member

Any updates?

@srawlins
Copy link
Member

Famous last words in February:

I'm getting my ducks in a row to land this.

I have hit another issue, #3748, and I believe that fixing #3748 will unblock a number of features that have been requested, including this one.

@bwilkerson
Copy link
Member

Any progress?

@srawlins
Copy link
Member

There has not been direct progress. I have only found more yaks to shave in order to implement this. I think it should still be P1. And I am working on the other yaks. I should try to link back here where applicable, to leave a paper trail.

@srawlins
Copy link
Member

Still making slow progress 😁 Removed duplicate notions of "canonical library" last week.

@srawlins
Copy link
Member

Still slow progress. But the work towards doc-imports actually helped move this forward a bit, as some bugs were discovered in how extensions were handled.

@srawlins
Copy link
Member

Sorry for the delay, all, but I have a CL that's just about ready to go. There are some design questions though, and I'd love any and all input. @kevmoo I'm looking at you. CC @parlough

We don't have to do a design research project, or get it perfect the first time. I'd love to move forward with something that works well enough, and doesn't use style that directly clashes with anything else on the site or our other properties.

Class body layout

On a "class page", put extension instance methods (and properties and operators) inline with "directly declare" instance methods? Or have a bunch of new sections, "Extension instance methods," "Extension instance properties," "Extension operators?" I vote the former.

And how can we distinguish that one is only available via an extension you may need to import? Or that one is only available on some subtypes? For example a field that is available on List (so should be listed on the List page), but actually only available on List<Enum> or Iterable<Future>? Here are some shots of what that can look like:

Screenshot 2024-08-27 at 1 48 06 PM
Screenshot 2024-08-27 at 1 48 21 PM

Sidebar layout

Should extension methods (and properties and operators) be inline with other instance members, or in separate sections? Again, I think inline is good. So how to distinguish? Here is one possibility, using simple asterisks and hover text:

Screenshot 2024-08-28 at 9 06 29 AM

@kevmoo
Copy link
Member

kevmoo commented Sep 5, 2024

Hrm. Interesting. My only (slight) concern is that this well only show...what...the default extensions available from the library in question?

Fill folks me confused why some show up and others don't (based on their imports)?

@munificent
Copy link
Member Author

I also vote for inline and like the "Available on ..., provided by the ... extension." design and links.

In the sidebar, maybe something like "(ext)"?

@srawlins
Copy link
Member

srawlins commented Sep 5, 2024

Hrm. Interesting. My only (slight) concern is that this well only show...what...the default extensions available from the library in question?

Actually, as I've implemented it, it's pretty wide-sweeping. So in the Dart SDK, we would cross-link any extensions declared anywhere in the Dart SDK (any public ones, that is). But in the Flutter SDK, it's wider: we would cross-link any extensions declared anywhere in any of the documented packages. Good? I... think so?

For example, the collection package is documented at api.flutter.dev. This package provides many extensions, like IterableIntegerExtension. So this extension exposes members to Iterable expressions. So on every page for a subtype of Iterable, like List, Queue, etc., even the Characters class from the characters package.

This is not out-of-line with other cases of this "universality" at api.flutter.dev, like how Iterable's "Implementors" include PathMetrics, CachingIterable, and Characters.

In the sidebar, maybe something like "(ext)"?

@pq suggested something similar, rather than the rather ambiguous asterisk. I will prototype something like a subscript "e" or "(ext)" or "(E)".

Thanks for everyone's input 🙏

srawlins added a commit to srawlins/dartdoc that referenced this issue Sep 6, 2024
Fixes dart-lang#2021

In this feature, we list extension members of an extended type on that extended
type's page. The members are listed inline with other instance members and are
cross-linked over to the extension's member pages.

* Introduce `availableInstanceMethods` as a list combining both instance
  methods declared in a container, and instance methods declared in an
  applicable extension. The same for `availableInstanceOperators` and
  `availableInstanceFields`.
* Introduce some constructors like `Field.providedByExtension` for members that
  are provided by an extension.
* Add lots of documentation clarifying _where_ various members come from.
* We introduce a new getter on ElementType, `nameWithGenericsPlain`, which is
  not HTML-formatted, for use in HTML "title text."
@mit-mit
Copy link
Member

mit-mit commented Sep 10, 2024

@srawlins is there a link to Flutter API docs that includes this so we can poke around a bit and see the result?

@goderbauer fyi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

8 participants