Skip to content

Add optional space prop for JSON.stringify#15

Merged
Eyas merged 3 commits intogoogle:masterfrom
ntucakovic:feature/14-add-stringify-space-prop
Apr 6, 2020
Merged

Add optional space prop for JSON.stringify#15
Eyas merged 3 commits intogoogle:masterfrom
ntucakovic:feature/14-add-stringify-space-prop

Conversation

@ntucakovic
Copy link
Contributor

Fixes #14

*/
export class JsonLd<T extends Thing> extends React.Component<{
item: WithContext<T>;
space?: string | number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you as a jsdoc above this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed a commit, is that what you had in mind or?

Copy link
Collaborator

Choose a reason for hiding this comment

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

My bad for not elaborating: you can put inline JSDocs above a type declaration (i.e. right above "space:") describing it, and it shows up in IDEs etc when someone's looking at props or seeing the autocomplete for space

Copy link
Contributor Author

@ntucakovic ntucakovic Apr 6, 2020

Choose a reason for hiding this comment

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

image

string | number is used as type declaration here instead of JSDoc, no? I don't think we can write JSDoc in inline interface declaration like in this case. IDE is complaining and TS won't compile. Can you check?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here's what I mean:

image

image

@ntucakovic ntucakovic requested a review from Eyas April 6, 2020 18:39
@Eyas Eyas merged commit 5c9c2c3 into google:master Apr 6, 2020
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.

Extend JsonLd component to accept stringify's space property

2 participants