ember-learn / ember-jsonapi-docs

fixes up the ember docs generated by YUIDoc to be in JSONAPI format + some other niceties
9 stars 11 forks source link

Bug - Parsing of new decorators doesn't work #75

Open jenweber opened 4 years ago

jenweber commented 4 years ago

In the latest work for Ember Octane, there are some new decorators that don't parse correctly. For example, if you use @tracked in a code sample, it doesn't render. However, we have used decorators in other parts of the guides successfully. It also seems that the processing itself hangs.

See https://github.com/ember-learn/ember-jsonapi-docs/blob/5c3d9d81107ef4ec8e1fe016de8be3c3c214121b/lib/fix-borked-yuidoc-files.js for how this was handled for other cases.

jenweber commented 4 years ago

Debugging strategy:

If the initial parsing does not succeed, you can comment out the work being done in a file called fix-borked-yuidocs.js or something like that. Then parsing will finish, and when you serve it locally, you will see broken code blocks wherever decorators are used

Overall flow:

Set the version number in ember.js package.json to something fake, like 4.0.0. Then...

cd ember-jsonapi-docs
rm -rf tmp
yarn gen --version 4.0.0 (pseudocode - see Readme for exact command)
yarn server
cd ../ember-api-docs
yarn start:local

Always remove tmp when you make changes.

MelSumner commented 4 years ago

@rwjblue @pzuraq @chancancode for visibility

sivakumar-kailasam commented 4 years ago

Digged into this a bit & figured that the reason this doesn't work is because the fix-borked-yuidoc-file script doesn't account for the decorators usage in the example yuidoc tag(when we authored that script, we didn't use example tags). Will add the fix shortly.

pzuraq commented 4 years ago

I'm working on a better fix for this upstream in ember-cli-yuidoc. The borked fix is alright, but it's still results in some data loss and messed up formatting, and is a bit hard to follow (but works pretty well all things considered!)

pzuraq commented 4 years ago

Fix is up here: https://github.com/cibernox/ember-cli-yuidoc/pull/52

sivakumar-kailasam commented 4 years ago

Thanks @pzuraq!

@rwjblue Once this fix is merged, can we release patch versions of ember & ember-data from 3.10-3.14 with the upgraded ember-cli-yuidoc? This fix would produce better formatted code blocks in the api docs & we'll no longer have to handle the messed up files in this app.

rwjblue commented 4 years ago

We can definitely get 3.12, 3.14, and 3.15-beta's out but I'm not sure about re-releasing patch releases of unsupported versions (3.10, 3.11, and 3.13 are all unsupported).

pzuraq commented 4 years ago

Could we possibly patch the docs directly, without doing a release? I'm not familiar with how the docs artifacts get generated and stored, but if ember-cli-yuidoc does a patch release, we shouldn't need to actually change anything in ember-source or ember-data, in theory, we could just regenerate the docs and publish those (which, tbh, might be a nice thing in general to be able to do for corrections/fixes)

sivakumar-kailasam commented 4 years ago

We source the yuidoc json files from npm via unpkg. Since this isn't a documentation issue but more of a tooling issue, I think I can work around it & make it work without having to release any patch version.

I'll generate the docs locally for each of those versions & use it in this app.

Regards, Siva

On Wed, Oct 30, 2019 at 9:17 PM Chris Garrett notifications@github.com wrote:

Could we possibly patch the docs directly, without doing a release? I'm not familiar with how the docs artifacts get generated and stored, but if ember-cli-yuidoc does a patch release, we shouldn't need to actually change anything in ember-source or ember-data, in theory, we could just regenerate the docs and publish those (which, tbh, might be a nice thing in general to be able to do for corrections/fixes)

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/ember-learn/ember-jsonapi-docs/issues/75?email_source=notifications&email_token=AAETPVJBYFWG2BQB5YFYZW3QRGUBBA5CNFSM4JFD6652YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECUWVXA#issuecomment-547973852, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAETPVMGZLD6NAUJ3ABRTSTQRGUBBANCNFSM4JFD665Q .

pzuraq commented 4 years ago

Could this be an issue if we ever need to regenerate those docs in the future? We should document what that process looks like, so we know about these versions of Ember where the docs need to manually generated. Either that, or potentially we should keep the other patch as well, specifically for those versions of docs.

sivakumar-kailasam commented 4 years ago

I'll document the process around this.

Although I'll generate the docs locally, I won't change the versions in those docs. So in case we release a newer patch version for any of those releases on npm, that'll be the one that'll be considered by this app.

Regards, Siva

On Wed, 30 Oct, 2019, 9:25 pm Chris Garrett, notifications@github.com wrote:

Could this be an issue if we ever need to regenerate those docs in the future? We should document what that process looks like, so we know about these versions of Ember where the docs need to manually generated. Either that, or potentially we should keep the other patch as well, specifically for those versions of docs.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/ember-learn/ember-jsonapi-docs/issues/75?email_source=notifications&email_token=AAETPVMTGUFXJNJ37EYQHX3QRGU5HA5CNFSM4JFD6652YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECUXSJQ#issuecomment-547977510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAETPVK2TMEWDZK6F6F47B3QRGU5HANCNFSM4JFD665Q .