Addon-docs/Angular: Fix default values in ArgsTable #15881
Conversation
Nx Cloud ReportCI ran the following commands for commit a2d0754. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch
Sent with 💌 from NxCloud. |
| @@ -157,7 +158,17 @@ export const extractType = (property: Property, defaultValue: any) => { | |||
| const extractDefaultValue = (property: Property) => { | |||
| try { | |||
| // eslint-disable-next-line no-eval | |||
| const value = eval(property.defaultValue); | |||
| let value = eval(property.defaultValue); | |||
There was a problem hiding this comment.
Can we just use the string for this and not eval anything? Eval was necessary when we needed the actual value, but it is a security risk and error prone, so it would be great if we could just skip it now that we only need a string.
There was a problem hiding this comment.
Definitely. I only left the eval in as I was not sure about backwards compatibility and what might break if removed.
There was a problem hiding this comment.
|
@shilman, eval removed. Note that this change also adds defaults to Outputs, something like this - |
| if (value == null && property.jsdoctags.length > 0) { | ||
| property.jsdoctags.forEach((tag: JsDocTag) => { | ||
| if (['default', 'defaultvalue'].includes(tag.tagName.escapedText)) { | ||
| const tmp = window.document.createElement('DIV'); |
There was a problem hiding this comment.
Is this just to unescape HTML? If so, is there a library we can use for this?
There was a problem hiding this comment.
I originally used DOMParser code below but husky pre-commit throws this error - 'DOMParser' is not defined.
const dom = new DOMParser().parseFromString(tag.comment, 'text/html');
value = dom.body.textContent;
If I use window.DOMParser then Typescript has errors. If I add // @ts-ignore then all is well. WDYT?
// @ts-ignore
const dom = new window.DOMParser().parseFromString(tag.comment, 'text/html');
value = dom.body.textContent;
There was a problem hiding this comment.
i like this better if it works 😁
There was a problem hiding this comment.
@shilman It's been done. Can we merge?
|
@dexster sorry for the slow reply on this. CI is failing --- are you able to see the failures? any idea what's going on? |
|
Looks like the user arg is being incorrectly set in the Angular11 e2e test:
|
shilman
changed the title
shilman
changed the title
Issue:
What I did
Added defaultValue to table.defaultValue.summary as per @shilman comments in #12644.
Also, added support for JSDoc @default and @DefaultValue to allow scenarios like accessor defaults where Compodoc will not find default values
How to test
Kitchen sink app has been updated to show defaultValues and also new accessor defaultValues