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

Quest - update API docs to show Octane and Native Classes #18250

Open
jenweber opened this issue Aug 10, 2019 · 5 comments

Comments

@jenweber
Copy link
Contributor

@jenweber jenweber commented Aug 10, 2019

It's almost time to show Octane-style code samples in the API docs! Right now, our examples use Ember Objects, and we want them to use native classes and Component file co-location instead. Can you help? Here's how! If you have any questions, drop by the #dev-ember-learning channel on Ember Discord. Keep reading for step by step instructions, a styleguide, and some examples of the kinds of changes we need help with.

Step by step instructions

  1. Comment on this issue to say which item you are volunteering for. The checklist of things we need help with is in the comment below. Only volunteer for items that you think you can open a PR for in a week or less.
  2. Before you start working, double check to make sure it's not already been PR'd or claimed by someone else.
  3. Fork this repo and check out the existing branch octane-docs. This branch has some of the work done already, and you will be adding to it. If it's your first open source PR ever, check out these instructions to learn how to fork, branch, etc. or make changes via the GitHub website instead.
  4. Locate the file that has the API doc. Our docs are written in YUIdoc and they live right next to the JavaScript for the API, here in this repository.
  5. Make your changes! EmberObject is still supported, so we need to show both styles. Here's an example. In general, you should give Octane examples first, and EmberObject second. In cases where multiple examples are needed, make all of them Octane, and then add just 1 for EmberObject.
    If you don't know Octane syntax yet, don't worry. There's a Cheatsheet and guide to what is different
  6. When you make a commit, it should start with [DOC beta]. For example [DOC beta] update component example for Octane.
  7. Open a pull request, and target merging into the octane-docs branch. Please check the box that says "allow maintainers to make edits."
  8. Celebrate and brag about it to all your friends

Styleguide

All inline documentation is written using YUIDoc. Follow these rules when updating or writing new documentation. This is copied & pasted from CONTRIBUTING.md

  • All code blocks must be fenced
  • All code blocks must have a language declared
  • All code blocks must be valid code for syntax highlighting
  • All code blocks should have an empty line before and after
  • All examples in code blocks must be aligned
  • All references to code words must be enclosed in backticks
  • Prefer a single space between sentences
  • Reference Ember.js as Ember.
  • Wrap long markdown blocks > 80 characters
  • Don't include blank lines after @param definitions

Example

Since this example is trying to show Templates and not teaching Component APIs, it's ok to show just Octane. We'll update the Component to use the native class syntax of @glimmer/component and change the filepaths to reflect co-location (where a component's hbs and js files are both side-by-side in the components folder. We also add this for component properties that belong to the component. @ is used in templates when it is assumed that a property is passed in from the parent._

Before:

Templates manage the flow of an application's UI, and display state (through
the DOM) to a user. For example, given a component with the property "name",
that component's template can use the name in several ways:

  import Component from '@ember/component';

  export default Component.extend({
    name: 'Jill'
  });
  {{name}}
  <div>{{name}}</div>
  <span data-name={{name}}></span>

After

Templates manage the flow of an application's UI, and display state (through
the DOM) to a user. For example, given a component with the property "name",
that component's template can use the name in several ways:

import Component from '@glimmer/component';

export default class PersonProfile extends Component {
   name = 'Jill';
}
  {{this.name}}
  <div>{{this.name}}</div>
  <span data-name={{this.name}}></span>
@jenweber

This comment has been minimized.

Copy link
Contributor Author

@jenweber jenweber commented Aug 10, 2019

We will try to keep this list up to date, but please double check comments below, plus open & merged PRs before beginning work.

Needs volunteers

The octane-docs branch has some of this work started, so double check to see if it has been done already!

  • @ember/component should have nearly all content removed except the link to the other component api page
  • This Component page needs a note about how @ember/components are referred to as "classic components."
  • Some examples on Controller need to be updated to use native classes. Others use classic component behavior.
  • write an isEqual example that does not use EmberObject.
  • Application really needs a note at the top that application code is generated for you in Ember and that many apps never
  • Controller actions need to use native class syntax and also show classic. Mixins should be moved way down.
  • action helper should probably have a link to {{on}} saying that it is an alternative. Also convert the Controller example in "specifying a target" to native classes
  • change app/templates/components/my-component.hbs to handlebars in the code block for has-block and hasBlockParams to fix the syntax highlighting
  • bugfix - input helper isn't showing angle brackets. Edit the angle brackets work that has already been done to also show some curly braces examples, and then delete the curly braces example docs. See #18125 for details.
  • same as above, for LinkTo and TextArea
  • mut maybe needs a note that it's for use with Classic components only, not @glimmer/component. I think???
  • Maybe partial should just state the alternative of deleting the js file. Does this need a flag? A little unsure of this so ask someone on the core team for clarification.
  • remove jquery from the onInjectHelpers example.
  • Helper examples need @ or this in front of the args. Should show a native class and classic example of the class-based Helper boilerplate.

Needs more info and blocked

These items need some more discussion or clarification before they can be worked on. People who know the answers, please move items from this list and make them to-dos above.

  • Should link to the glimmer component docs in a bunch of places, once we have them.
  • How to document Controllers and Routes well? A bunch of the APIs are more about classic than Octane, so maybe the right move here is to show the boilerplate of classic and Octane at the top.
  • Engines, how do they work?
  • We need some kind of warnings or caveats around mixins so people don't try to use them on glimmer components
  • how to document observers and computeds as decorators properly?

WIP

  • almost every example in Route needs to be native class'ed. Should show native and classic both at the very top.

Done

  • show native class controller in this templates example
  • jquery $ maybe needs a note about how jquery is not included by default, and links to https://guides.emberjs.com/release/configuring-ember/optional-features/
  • the RouterService example should use @glimmer/component syntax
  • urlFor should use a native class example.
  • the component example for the component helper should use @glimmer/component in addition to classic. Also refactor the example to not use computed
  • the examples of the each helper need this. or @ in front of some of the args
  • the examples of the each-in helper need this. or @ in front of some of the args
  • the examples of the get helper need this. or @ in front of some of the args
  • the link-to example for query params should use angle bracket syntax, and it needs an @ or this in front of the args
  • unbound needs an @ or this in front of the args. Does it work with @glimmer/components???
  • if need this. or @ in front of some of the args
  • unless needs an @ or this in front of the args.
  • with needs an @ or this in front of the args.
  • yield needs an @ or this in front of the args and the examples should use angle brackets.
  • the outlet Route example needs to be a native class.
@locks locks mentioned this issue Aug 11, 2019
1 of 17 tasks complete
@locks

This comment has been minimized.

Copy link
Contributor

@locks locks commented Aug 11, 2019

  • Should we suggest an alternative to named outlets?
  • Some examples on Controller need to be updated to use native classes. Others use classic component behavior. Does this mean that we keep the classic behavior examples as they are?
  • Does it make sense not to use EmberObject in isEqual examples? Should we instead using a third-party library for equality?
  • Application … and that many apps never need to touch it? Should we not generate it by default?
  • Controller actions need to use native class syntax and also show classic is from the action_handler mixin. We might need a deeper approach here since it impacts a couple of different classes.
  • the examples of … need this or @ in front might not be specific enough. should there be a preference for one or the other? should we purposefully mix them?
  • Maybe partial should just state the alternative of deleting the js file. It might be a better approach to document how to migrate from partials to template-only components and link to that. Linking to a mention of template-only components in the guides would also be useful, but not mentioning that you have to explicitly pass all arguments and update the partial template with the @ prefix might lead the dev to headaches.
  • almost every example in Route needs to be native class'ed. Should show native and classic both at the very top. Do you mean one single example so readers have a sense how things map?
@MelSumner

This comment has been minimized.

Copy link
Contributor

@MelSumner MelSumner commented Aug 20, 2019

@rwjblue could you take a peek at this issue? Can you provide any clarity for @locks question?

@locks

This comment has been minimized.

Copy link
Contributor

@locks locks commented Aug 31, 2019

Missing

  • what to do with mut in get documentation
  • bring back classic component example for component helper
  • normalize set and tracked in computed_macros file
  • review each-in for correctness/coesion (key is used before it's introduced)
  • confirm get + mut example updates properly
  • action_handler switch examples to EmberComponent so that we can still use the classic .extend API.
  • investigate syntax highlighting mishaps - hasBlock, hasBlockParams
  • observer in Helper class example
  • RouterService#recognize uses a strange ember component

Missing code samples

  • RouterService#isActive
  • RouterService#recognizeAndLoad

Open questions

  • classes, named or anonymous? (class extends Route vs class Menu extends Route) -> named

Router Service

  • constructor seems ok in code samples? (cc @pzuraq)
  • currentRoute example is using notEmpty CP decorator. should we replace with native getter? (cc @pzuraq)
    • Yes, let's use native getters

unless

  • Consider introducing not to the framework, and deprecating unless for {{if (not)}}
    • this is out of scope

Action Handler

  • (?) action_handler remove mixins (?)

Controller

  • figure out what to do with Controller#addObserver - it's currently using an EmberComponent, which seems strange

action helper

  • figure out how we want to address classic EmberComponent in action helper

partial

While the partial deprecation is not currently implemented, it is possible that Octane will ship with it.
Regardless, we should definitely suggest GlimmerComponents instead.

Helper (class)

  • @observer?

    • With autotracking the examples for these docs shouldn't need observers:
    import Helper from '@ember/component/helper';
    import { inject as service } from '@ember/service';
    import { get } from '@ember/object';
    
    export default class UserEmail extends Helper {
      @service() session;
    
      compute() {
        return get(this, 'session.currentUser.email');
      }
    }

Route

  • on('activate', function(){ + native class?
  • willTransition, didTransition, loading, error actions + native class?
@jenweber

This comment has been minimized.

Copy link
Contributor Author

@jenweber jenweber commented Sep 3, 2019

@locks regarding:

figure out what to do with Controller#addObserver - it's currently using an EmberComponent, which seems strange

This came up in the octane meeting. We should change the example to not use a component, and we should add some boilerplate that "addObserver is a low-level feature that is not performant" or something similar. If possible, we should change the module so it doesn't say "component."

For action, we should link to the guides for usage and remove all "guides-like" content. The actions page should focus on the API surface for classic component actions. It needs a note about how this works in native class components. We may also document part of the glimmer component usage. @tomdale will take a closer look to see what can be done.

@acorncom acorncom added the Quest label Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.