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

[FIX DOCS] Docs updates for Packages to fix missing docs and broken links #6449

Merged
merged 9 commits into from
Sep 27, 2019

Conversation

runspired
Copy link
Contributor

@runspired runspired commented Sep 13, 2019

should attempt backport to BETA and RELEASE

This introduces a fake model_package_named_exports "class" (which will appear both in the left sidebar under classes and in the @ember-data/model module as a class) in order to restore access to the documentation for attr belongsTo and hasMany.

An alternative is to leave their documentation attached to a global DS class.

This is necessary because yuidocs does not have a concept of standalone methods. All methods must be attached to a class.

We unfortunately cannot use @ember-data/model as a fake class (I tried) because yuidocs justifiably considers this to not be a valid class name.

A few additional things I discovered in this process that are limitations of yuidoc

  • decorators cause doc comments to be immediately ended
  • sub-packages can't be documented
  • if a package is referenced by another package it creates an extra class in the docs by accident
  • module definitions link to whatever module is last seen by the yui-parser and not the first file nor the file that defines @main
  • no ability to document methods as "optional"

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue 🏷️ doc This PR adds/improves/or fixes documentation 🎯 beta PR should be backported to beta 🎯 canary PR is targeting canary (default) 🎯 release PR should be backported to release labels Sep 13, 2019
* [normalize](/api/data/classes/DS.EmbeddedRecordsMixin.html#method_normalize)
* [serializeBelongsTo](/api/data/classes/DS.EmbeddedRecordsMixin.html#method_serializeBelongsTo)
* [serializeHasMany](/api/data/classes/DS.EmbeddedRecordsMixin.html#method_serializeHasMany)
* [normalize](/api/data/classes/EmbeddedRecordsMixin.html#method_normalize)
Copy link
Contributor Author

@runspired runspired Sep 13, 2019

Choose a reason for hiding this comment

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

these links are likely still broken (and were likely broken before) as the API docs url structure is not /api/data/classes/ but /ember-data/release/classes/ and do not contain .html endings. I think anchors are also done via ?anchor= now?

Copy link

Choose a reason for hiding this comment

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

Correct, any sub header I navigate to uses ?anchor, e.g. registerForType

@runspired
Copy link
Contributor Author

CC @efx @Gaurav0 @jenweber

By default, attributes are passed through as-is, however you can specify an
optional type to have the value automatically transformed.
Ember Data ships with four basic transform types: `string`, `number`,
`boolean` and `date`. You can define your own transforms by subclassing
[Transform](/api/data/classes/DS.Transform.html).
[Transform](/api/data/classes/Transform.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -27,12 +27,12 @@ function hasValue(internalModel, key) {
}

/**
`attr` defines an attribute on a [Model](/api/data/classes/DS.Model.html).
`attr` defines an attribute on a [Model](/api/data/classes/Model.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -10,7 +10,7 @@ import { computedMacroWithOptionalParams } from './util';

/**
`belongsTo` is used to define One-To-One and One-To-Many
relationships on a [Model](/api/data/classes/DS.Model.html).
relationships on a [Model](/api/data/classes/Model.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -457,7 +457,7 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
key. Arbitrary headers can be set as key/value pairs on the
`RESTAdapter`'s `headers` object and Ember Data will send them
along with each ajax request. For dynamic headers see [headers
customization](/api/data/classes/DS.RESTAdapter.html).
customization](/api/data/classes/RESTAdapter.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -9,7 +9,7 @@ import { computedMacroWithOptionalParams } from './util';

/**
`hasMany` is used to define One-To-Many and Many-To-Many
relationships on a [Model](/api/data/classes/DS.Model.html).
relationships on a [Model](/api/data/classes/Model.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -918,7 +918,7 @@ abstract class CoreStore extends Service {
### Retrieving Related Model Records

If you use an adapter such as Ember's default
[`JSONAPIAdapter`](https://emberjs.com/api/data/classes/DS.JSONAPIAdapter.html)
[`JSONAPIAdapter`](https://emberjs.com/api/data/classes/JSONAPIAdapter.html)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -1844,7 +1844,7 @@ abstract class CoreStore extends Service {
```

This method returns a promise, which is resolved with an
[`AdapterPopulatedRecordArray`](https://emberjs.com/api/data/classes/DS.AdapterPopulatedRecordArray.html)
[`AdapterPopulatedRecordArray`](https://emberjs.com/api/data/classes/AdapterPopulatedRecordArray.html)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -2172,7 +2172,7 @@ abstract class CoreStore extends Service {
### Retrieving Related Model Records

If you use an adapter such as Ember's default
[`JSONAPIAdapter`](https://emberjs.com/api/data/classes/DS.JSONAPIAdapter.html)
[`JSONAPIAdapter`](https://emberjs.com/api/data/classes/JSONAPIAdapter.html)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

@@ -152,12 +136,12 @@ class Store extends CoreStore {

The class of a model might be useful if you want to get a list of all the
relationship names of the model, see
[`relationshipNames`](https://emberjs.com/api/data/classes/DS.Model.html#property_relationshipNames)
[`relationshipNames`](https://emberjs.com/api/data/classes/Model.html#property_relationshipNames)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link likely still broken

* [isDeleted](DS.Model.html#property_isDeleted)
* [isNew](DS.Model.html#property_isNew)
* [isValid](DS.Model.html#property_isValid)
* [isEmpty](Model.html#property_isEmpty)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

links to all state flags likely still broken

@Gaurav0
Copy link
Contributor

Gaurav0 commented Sep 14, 2019

Current Master:

image

This PR:

image

Overall, while I think this PR is an improvement, there are two issues:

  1. It shows a default class for model and store modules but not adapter and serializer modules, which should be fixed. Also, Ember.js modules do not have default classes. I personally think default classes (@main) should not be provided, because I would rather see only lists of classes and functions when clicking on a module.

  2. I think we should stick with @ember/data/model as the "class name" for functions not in a class, rather than create a fake class name. This is consistent with how it is being done with Ember.js functions.

To see results in your local machine, follow instructions here

* fix links to classes

* revert named_exports fake class

* fix VERSION doc is missing
@ghost
Copy link

ghost commented Sep 18, 2019

@Gaurav0 thank you for linking to the the other steps to build this locally. I'm running into some errors while trying to build just ember-data's API documentation. Could you post the shell commands you ran to get it working?

I had tried yarn gen --project ember-data --version master and yarn gen --project ember-data --version 3.14.0-alpha.3.da27f67a.

@Gaurav0
Copy link
Contributor

Gaurav0 commented Sep 18, 2019

Keeping in mind that ember repo must be in ../ember.js, ember-data repo must be in ../data, and ember-api-docs repo in ../ember-api-docs, and that yarn install has been run in all the 4 required repos,

export AWS_ACCESS_KEY_ID=xxxx
export AWS_SECRET_ACCESS_KEY=xxxxxx
yarn gen --project ember-data --version 3.14.0
yarn server

Then in another terminal, in ember-api-docs directory

yarn start:local

Feel free to post the errors here and I'll try to help.

@sivakumar-kailasam
Copy link
Member

sivakumar-kailasam commented Sep 19, 2019 via email

@ghost
Copy link

ghost commented Sep 19, 2019

Thanks @Gaurav0 and @sivakumar-kailasam I am able to see the docs now. I tried the same commands from yesterday and it works today! I am only seeing 3.12 as a possible version in the left dropdown though. I also want to automate the initial repository setup as a yarn script in ember-learn/ember-jsonapi-docs.

@sivakumar-kailasam
Copy link
Member

You're seeing only 3.12 docs generated since you would've passed the version param. 

@Gaurav0
Copy link
Contributor

Gaurav0 commented Sep 21, 2019

@runspired Can we target 3.12 LTS as well?

@runspired runspired added 🎯 lts The PR should be backported to the most recent LTS target:lts-3-12 labels Sep 25, 2019
@runspired
Copy link
Contributor Author

I'd love to get all the links working in this PR before we merge it, that way we can fix all the various packages releases (3.12 LTS -> 3.15 canary)

Copy link
Contributor Author

@runspired runspired left a comment

Choose a reason for hiding this comment

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

Just audited, I think this is largely as good as it can be until we can test locally easily or deploy it. I think we should ship this and do any additional links fixes in follow up, as even if a few are still broken most will now work where they don't currently.

@runspired runspired merged commit bc4c5a4 into master Sep 27, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/ds-docs branch September 27, 2019 06:34
Gaurav0 added a commit to Gaurav0/data that referenced this pull request Sep 27, 2019
…inks (emberjs#6449)

* remove ember-data and DS namespace/module
* eliminate usage of DS.Store in the documentation
* improve Store documentation
* configure Store docs to be the main docs for the @ember-data/store package
* cleanup Model docs
* fix adapter/serializer location and main docs
* cleanup DS. usage in docs
* Fix/ds docs (emberjs#6456)
* [DOCS] Fix some links (emberjs#6507)

Co-Authored-By: Gaurav Munjal <Gaurav0@aol.com>
@Gaurav0 Gaurav0 mentioned this pull request Sep 27, 2019
Gaurav0 added a commit to Gaurav0/data that referenced this pull request Sep 27, 2019
…inks (emberjs#6449)

* remove ember-data and DS namespace/module
* eliminate usage of DS.Store in the documentation
* improve Store documentation
* configure Store docs to be the main docs for the @ember-data/store package
* cleanup Model docs
* fix adapter/serializer location and main docs
* cleanup DS. usage in docs
* Fix/ds docs (emberjs#6456)
* [DOCS] Fix some links (emberjs#6507)

Co-Authored-By: Gaurav Munjal <Gaurav0@aol.com>
Gaurav0 added a commit to Gaurav0/data that referenced this pull request Sep 27, 2019
…inks (emberjs#6449)

* remove ember-data and DS namespace/module
* eliminate usage of DS.Store in the documentation
* improve Store documentation
* configure Store docs to be the main docs for the @ember-data/store package
* cleanup Model docs
* fix adapter/serializer location and main docs
* cleanup DS. usage in docs
* Fix/ds docs (emberjs#6456)
* [DOCS] Fix some links (emberjs#6507)

Co-Authored-By: Gaurav Munjal <Gaurav0@aol.com>
runspired pushed a commit that referenced this pull request Sep 27, 2019
…inks (#6449) (#6522)

* remove ember-data and DS namespace/module
* eliminate usage of DS.Store in the documentation
* improve Store documentation
* configure Store docs to be the main docs for the @ember-data/store package
* cleanup Model docs
* fix adapter/serializer location and main docs
* cleanup DS. usage in docs
* Fix/ds docs (#6456)
* [DOCS] Fix some links (#6507)

Co-Authored-By: Gaurav Munjal <Gaurav0@aol.com>
runspired pushed a commit that referenced this pull request Sep 27, 2019
* [FIX DOCS] Docs updates for Packages to fix missing docs and broken links (#6449)

* remove ember-data and DS namespace/module
* eliminate usage of DS.Store in the documentation
* improve Store documentation
* configure Store docs to be the main docs for the @ember-data/store package
* cleanup Model docs
* fix adapter/serializer location and main docs
* cleanup DS. usage in docs
* Fix/ds docs (#6456)
* [DOCS] Fix some links (#6507)

Co-Authored-By: Gaurav Munjal <Gaurav0@aol.com>

* fix lint
runspired pushed a commit that referenced this pull request Sep 27, 2019
* [FIX DOCS] Docs updates for Packages to fix missing docs and broken links (#6449)

* remove ember-data and DS namespace/module
* eliminate usage of DS.Store in the documentation
* improve Store documentation
* configure Store docs to be the main docs for the @ember-data/store package
* cleanup Model docs
* fix adapter/serializer location and main docs
* cleanup DS. usage in docs
* Fix/ds docs (#6456)
* [DOCS] Fix some links (#6507)

Co-Authored-By: Gaurav Munjal <Gaurav0@aol.com>

* fix lint
@igorT igorT removed 🎯 beta PR should be backported to beta 🎯 release PR should be backported to release labels Nov 4, 2019
@HeroicEric HeroicEric removed 🎯 canary PR is targeting canary (default) 🎯 lts The PR should be backported to the most recent LTS target:lts-3-12 labels Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue 🏷️ doc This PR adds/improves/or fixes documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants