feat: custom extension rendering #994
Conversation
ductaily
requested review from
magicmatatjahu,
derberg,
fmvilas and
asyncapi-bot-eve
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
ductaily
changed the title
There was a problem hiding this comment.
I assume that docs, test and playground showcase is not here yet as you first what to have us look at implementation to give green light for more?
than please at least extend PR with default extensions components that are supported by react component by default? probably best if they are in new dir like /library/src/components/extensions. You can take x-x and x-linkedin as first official extensions under info object
|
@ductaily Hi! Great work, but you shouldn't override the logic for |
Hi, thank you for reviewing. import React, { useContext, useState } from 'react';
import { Schema } from './Schema';
import { SchemaHelpers } from '../helpers';
import { AsyncAPIDocumentInterface } from '@asyncapi/parser';
import { useConfig, useSpec } from '../contexts';
import { CollapseButton } from './CollapseButton';
interface Props {
name?: string;
item: any;
}
export interface ExtensionComponentProps<V = any> {
propertyName: string;
propertyValue: V;
document: AsyncAPIDocumentInterface;
}
const SchemaContext = React.createContext({
reverse: false,
});
export const Extensions: React.FunctionComponent<Props> = ({
name = 'Extensions',
item,
}) => {
const { reverse } = useContext(SchemaContext);
const [expanded, setExpanded] = useState(false);
const [deepExpand, setDeepExpand] = useState(false);
const config = useConfig();
const document = useSpec();
const extensions = SchemaHelpers.getCustomExtensions(item);
if (!extensions || !Object.keys(extensions).length) {
return null;
}
if (!config.extensions || !Object.keys(config.extensions).length) {
const schema = SchemaHelpers.jsonToSchema(extensions);
return (
schema && (
<div className="mt-2">
<Schema schemaName={name} schema={schema} onlyTitle={true} />
</div>
)
);
}
return (
<div>
<div className="flex py-2">
<div className="min-w-1/4">
<>
<CollapseButton
onClick={() => setExpanded(prev => !prev)}
expanded={expanded}
>
<span className={`break-anywhere text-sm ${name}`}>{name}</span>
</CollapseButton>
<button
type="button"
onClick={() => setDeepExpand(prev => !prev)}
className="ml-1 text-sm text-gray-500"
>
{deepExpand ? 'Collapse all' : 'Expand all'}
</button>
</>
</div>
</div>
<div
className={`rounded p-4 py-2 border bg-gray-100 ${
reverse ? 'bg-gray-200' : ''
} ${expanded ? 'block' : 'hidden'}`}
>
{Object.keys(extensions).map(extensionKey => {
if (config.extensions && config.extensions[extensionKey]) {
const CustomExtensionComponent = config.extensions[extensionKey];
return (
<CustomExtensionComponent
propertyName={extensionKey}
propertyValue={extensions[extensionKey]}
document={document}
/>
);
} else {
const extensionSchema = SchemaHelpers.jsonToSchema(
extensions[extensionKey],
);
return (
<div className="mt-2">
<Schema schemaName={extensionKey} schema={extensionSchema} />
</div>
);
}
})}
</div>
</div>
);
};It would duplicate lots of logic of the Schema component. |
|
Hello, @ductaily! 👋🏼 |
|
/please-take-a-look |
|
@fmvilas @magicmatatjahu Please take a look at this PR. Thanks! 👋 |
|
@ductaily Hi! From the perspective of code maybe it looks like the code duplication, but extensions are not a "extended" schemas, but separate sections of spec, so even if in code it's better to have new logic inside Schema component, from spec perspective (and user) is better to have separate component. Also, if you will add logic in Schema component, new logic will be executed for all schemas in spec. Atm we treat extensions as schemas - we don't have logic to render extensions in custom way so it was ok, now it is not. Also, some people uses the Schema component outside the AsyncAPI component in their code, so using Next thing, I added (in my comment #819 (comment)) Also as I know, you can have a problem with circular references between extension. It's not a problem for extension because if extension allows to have circular refs, then author for custom component should focus on resolving that problem - still if extension won't have a custom component, then Schema component should render it and it should be handled (I see in your new example, and you handled that case). A couple of comments for the new code:
If you will have a problems, lets us know! |
I would oppose to this requirement. It's better to sort extensions by name (alphabetically) and render in that order. In our use case (SAP) we have some fields with shared prefixes. Some of those fields will have a custom component and some won't. It would mean they will be rendered apart from each other. Example:
Expected behaviour: both fields are rendered close to each other in UI. |
…er' into feat/custom-extension-value-render # Conflicts: # library/src/components/Extensions.tsx
|
@pavelkornev HI! Sorry for delay!
Ok, I understand :) |
Co-authored-by: Maciej Urbańczyk <[email protected]>
@derberg I will add the docs, test and playground showcase. Regarding your second comment just to clarify. ...
extensions: {
'x-x': DefaultExtensionComponent,
'x-linkedin': DefaultExtensionComponent,
},Having something like export default function DefaultExtensionComponent(
props: ExtensionComponentProps,
) {
return (
<div className="mt-2">
<div className="flex py-2">
<div className="min-w-1/4 mr-2">
<span className="break-anywhere text-sm italic">
{props.propertyName}
</span>
</div>
<div>
<div className="text-sm prose">{props.propertyValue}</div>
</div>
</div>
</div>
);
}Is that correct? |
|
@ductaily not default component, but specific component for each extension, just like you will have probably some specific component for so in case of so they are default in a way they are included in the library, but still custom, if you know what I mean. great is that they will be a nice, production showcase how to add extensions |
|
@derberg you mean to keep all extension components within this repo? We have use cases where we would need to set internal links, therefore we cannot commit such components to this public repo. I think both use cases can co-exist. |
Hi @derberg, @kennethaasan, Could you please check the following:
|
|
You're right, it stopped working. I'm pretty sure I checked this when we migrated from lerna to npm workspaces, but I guess some old linking was still in place on my machine. It looks like we need to change the |
|
@kennethaasan you're completely right. Tbh I though that for playground we did it already as anyway playground is not released to npm. Btw, @char0n that wrote the article that you linked, is maintainer of asyncapi spec and also uses this react component in swagger editor 😄 small world 😄 @ductaily please do the following:
|
|
haha, very small world. And thanks for fixing this issue! |
|
@derberg @kennethaasan Thanks a lot for your help :)
|
|
|
# Conflicts: # package-lock.json
…er' into feat/custom-extension-value-render
|
sorry for being late, we had some issues. Me and few other maintainers lost funding https://www.asyncapi.com/blog/2024-june-summary#postman-partnership-changes. Please join AsyncAPI Slack where you can ping me when you see an unusual delay on review
just add example also, once you do it, please add a screen shot to show where it can be found. I have to admit I quickly added it and could not locate where it is rendered |
Hey @derberg, thank you for the guidance :) 
|
|
There was a problem hiding this comment.
the only problem is that x-x extension should be used only in info object level, not channel
https://github.com/asyncapi/extensions-catalog/blob/master/extensions/x.md
@magicmatatjahu can you also have a look please
Custom extensions from Info block are not rendered by UI at all. We can replace the example with some other custom field. Which field can you recommend to use instead as an example? |
|
Description
Changes proposed in this pull request:
Example
Related issue(s)
See also #819