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

Documenting Components #678

Closed
wants to merge 2 commits into from
Closed

Conversation

gossi
Copy link

@gossi gossi commented Oct 27, 2020

Copy link
Contributor

@jelhan jelhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very excited by this RFC! Thanks a lot for writing it.

Settling on a shared standard to document components API will be very helpful for the community in my opinion. I expect it to be adopted by existing and new tools quickly as typed templates, improved autocompletion in editiors and automatically generated API docs will improve developer experience a lot.


### Documentation API

Documentation will use the powerful typing system of TypeScript. For TypeScript this works out of the box. For template-only components, use a `my-component.d.ts` file. For JavaScript, you can choose between a `*.d.ts` file or jsdoc with extended tsdoc annotations (see examples below).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is TSDoc supported for TypeScript as well? I think it would be helpful for at least three things:

  1. Descriptions as shown in the example given.
  2. To mark an argument or a yielded value as deprecated.
  3. To document default values.

I know that @glimmer/component doesn't have default values strictly speaking but there are many patterns, which create default values de facto. Like a getter which returns either the passed in argument or a default value if that one is undefined. Or a pattern like {{if @foo @foo "default"}} in the template.

I assume that TSDoc can always be used in TypeScript. But I think it would be good to have a shared agreement on what should be supported by tools implementing support for this RFC.

A similar question applies to JSDoc with extended tsdoc annotations: What JSDoc features are supported? Where do I find a specification and documentation for the supported tsdoc annotations?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, there are two things to that:

  1. Typescript language features as constructs for structured data
  2. The doc dialect

Ad 1)

The goal is to reach for structured data and typescript is helping here. For non-native-typescript support, TS offers a dialect addition for (2), see here: https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#typedef-callback-and-param (which is required to "upgrade" js to understand ts language features).

Ad 2)

The doc dialect is not part of this RFC. I believe this is something the community will settle on. There exists tsdoc, api-documentor (based on tsdoc) and jsdoc.
Apart from the requirements for js under (1), I think you are free to choose. At least from this RFC perspective. I have a feeling addons will rise for this ;)

@knownasilya
Copy link
Contributor

cc @lifeart

@pzuraq
Copy link
Contributor

pzuraq commented Jan 31, 2021

We discussed this RFC at the core team meeting on Friday, and were generally in favor of this direction in the long run. Using TypeScript and type systems in general to generate API documentation seems like a great innovation, and something we would like to support in Ember.

The issue lies in Ember also wanting to actually support TypeScript in the near future. The Typed Ember team is currently building out support for typed templates, and we're getting closer to being able to add official support in Ember itself, and to ship types with Ember. In order to do this, we will need to nail down the types for arguments, blocks, and attributes, and the chief concern here is that if we were to jump ahead and standardize types for documentation purposes alone, we may end up shipping types that conflict with actual TS support later on.

We are already in this situation a bit with @glimmer/component. Since Glimmer.js itself shipped with types in v1, Glimmer components did as well, and the typed-Ember community has built up around those types. Changing this would already introduce breakage, but expanding the scope of those types could introduce even more, so for the time being, until the larger TS story is figured out, we don't think it would be a good idea to expand the types for documentation purposes.

All that said, we do still think that this RFC could advance, if it were to shift focus a bit.

All components in Ember do a public API, which includes:

  1. Arguments, both named and positional
  2. Blocks, which are like callbacks that receive arguments (block params)
  3. Attributes, and the element they are applied to

Today, we don't really teach this as the public API in any consolidated location, and we don't guide users to document these things at all. It would be good for us to add this definition, along with a detailed explanation of each, to the guides.

We could also add some unopinionated documentation tooling that does not rely on types for the time being. For instance, we could add a generator that creates a my-component.md file, which includes a description of the args, blocks, and attributes/element that the component exposes. This would start us toward a more unified documentation story in general, and when the time comes to actually ship types with Ember, we would be a better place to ship types and tooling for components.

export default MyComponent extends Component<Input, Output> {}
```

The narrative is to give developers documentation slots for `Input` and `Output`. The `Input` will continue to work as is to document arguments and `Output` will be added to document blocks with their parameters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that Input and Output is the correct framing here. Blocks aren't exactly outputs, or at least, they are not the sole outputs of the component. The entire template within the component is output.

Blocks are more like callbacks in JavaScript. For instance:

this.store.fetchRecord('foo', 123).then(() => {
  // do things...
});

The callback passed to this promise is similar to a block. It gets passed as an argument, and internally it gets called later on at some point, and its result is slotted into a specific place. This is basically what blocks do as well.

To me, blocks are just a different form of input, namespaced so that they cannot collide with arguments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mental model preceeds blocks. Not accurate any longer. Agree, callbacks are more the mechanics that reflects this.

@lukemelia
Copy link
Member

lukemelia commented Jan 31, 2021

In evaluating an approach to documentation, it would be valuable to consider the path to expose documentation to javascript at runtime so that addons can use it to visualize the docs.

We're currently giving ember-freestyle the ability to document components with interactive controls. Here's the PR, which includes a short video of the feature in action: chrislopresto/ember-freestyle#457

Also worth noting: we've been co-locating the file defining the freestyle example alongside the component file with the name usage.hbs. This pattern is working out pretty nicely so far.

It would great to be able to a) automatically consume the component documentation in the usage file to create the interactive widget without repeating the documentation, and/or b) have a generator that could generate a base usage.hbs for a component based on the docs that is ready for customization.

@pzuraq
Copy link
Contributor

pzuraq commented Jan 31, 2021

@lukemelia so this is not necessarily the core team's view, but personally I really have liked the potential of Storybook.js for doing exactly that. I built a few proof of concepts about a year ago that exposed demo components using the controls addon from Storybook, which were really great and easy to use.

Because Storybook is super pluggable too, it would be possible to write a plugin that reads TS types of JSDoc comments and creates a tab with API documentation for the component. This is also part of the motivation behind the recent Argument Validation Primitives RFC, the thinking being that if we can make a statically analyzable high level format, then you could parse that straight into documentation via an addon. In addition, I created a .glimmer.md style file format which could be used to make interactive guides-style documentation, similar to what is available with ember-cli-addon-docs. This was all in an afternoon of hacking, so there was a very small amount of investment, with a very large amount of return.

The key issues are around how Ember integrates with Storybook currently, there are a number of issues (for instance, you have to boot up a whole Ember app for each example). But the changes we're working on for strict mode are bring us closer to being able to integrate very smoothly with it.

@josemarluedke
Copy link
Sponsor

I'm really looking forward to being able to further extract information from Components to generate documentation. Today that's already possible when authoring in TypeScript (for components args), and the addition of Argument Validation Primitives would allow extracting that when authoring in JS as well. What is missing is the story for blocks, and that's where I think this RFC helps a lot.

I have been working on a set of documentation tools and one of them is glimmer-docgen-typescript. This tool uses TypeScript to figure out all the arguments in a Glimmer Component. This tool could be modified to allow for blocks as well on that story if defined.

Additionally, I'm very much looking to the day we have typed templates, so if this RFC is a dependency for that, I would love to get it right to avoid breaking changes.

Note: You can take a look at Frontile's documentation to see glimmer-docgen-typescript in action.
On a side note about demos in markdown, checkout out Docfy for Ember (Writing Demos).

@gossi
Copy link
Author

gossi commented Feb 6, 2021

Thanks @pzuraq for your answer. From what I see, this RFC is already including a way for blocks, but is missing on attributes and element(s) and additionally even enhanced with argument validation.

What's best to advance from here?

@gossi
Copy link
Author

gossi commented May 13, 2021

Thanks everyone for the feedback. I gonna close this one as this has moved over to #748.

@gossi gossi closed this May 13, 2021
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

Successfully merging this pull request may close these issues.

None yet

6 participants