asyncapi-react icon indicating copy to clipboard operation
asyncapi-react copied to clipboard

feat: custom extension rendering

Open ductaily opened this issue 1 year ago • 30 comments

Description

Changes proposed in this pull request:

  • Extends the Extension component
  • Allows customized rendering of extensions passed as configuration

Example

import * as React from "react";
import { render } from "react-dom";
import AsyncApiComponent, { ConfigInterface } from "@asyncapi/react-component";
import { ExtensionComponentProps }  from "@asyncapi/react-component/src/components/Extensions"

const schema = `
asyncapi: '2.0.0'
info:
  title: Example
  version: '0.1.0'
channels:
  example-channel:
    subscribe:
      message:
        payload:
          type: object
          x-custom-extension: 'Customized Extension'
          properties:
            exampleField:
              type: string
            exampleNumber:
              type: number
            exampleDate:
              type: string
              format: date-time
`;

function MyCustomExtensionRender(props: ExtensionComponentProps) {
  return (
    <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 className="text-sm prose">
        <a target="_blank" href="#">
          {props.propertyValue}
        </a>
      </div>
    </div>
  )
}

const config: ConfigInterface = {
  schemaID: 'custom-spec',
  show: {
    operations: false,
    errors: false,
  },
  extensions: {
  'x-custom-extension': MyCustomExtensionRender
 }
};

const App = () => <AsyncApiComponent schema={schema} config={config} />;

render(<App />, document.getElementById("root"));

Bildschirmfoto 2024-04-22 um 18 44 26

Related issue(s)

See also https://github.com/asyncapi/asyncapi-react/issues/819

ductaily avatar Apr 22 '24 16:04 ductaily

@ductaily Hi! Great work, but you shouldn't override the logic for Schema component, but for Extensions like in my https://github.com/asyncapi/asyncapi-react/issues/819#issuecomment-1867433195 comment. By this you will be able to add extension not only for schemas but also for another sections of spec with possibility to define extensions, like channels, operations etc.

magicmatatjahu avatar Apr 23 '24 10:04 magicmatatjahu

@ductaily Hi! Great work, but you shouldn't override the logic for Schema component, but for Extensions like in my #819 (comment) comment. By this you will be able to add extension not only for schemas but also for another sections of spec with possibility to define extensions, like channels, operations etc.

Hi, thank you for reviewing. We don't understand the difference between our changes and how it would if we made the changes in the Extensions.tsx component.

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. Should we have a call?

ductaily avatar Apr 24 '24 09:04 ductaily

Hello, @ductaily! 👋🏼

    I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!

    At the moment the following comments are supported in pull requests:

    - `/please-take-a-look` or `/ptal` - This comment will add a comment to the PR asking for attention from the reviewrs who have not reviewed the PR yet.
    - `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
    - `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
    - `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR. (Currently only works for upstream branches.)
    - `/update` or `/u` - This comment will update the PR with the latest changes from the target branch. Unless there is a merge conflict or it is a draft PR. NOTE: this only updates the PR once, so if you need to update again, you need to call the command again.

asyncapi-bot avatar Apr 24 '24 09:04 asyncapi-bot

/please-take-a-look

ductaily avatar Apr 24 '24 09:04 ductaily

@fmvilas @magicmatatjahu Please take a look at this PR. Thanks! :wave:

asyncapi-bot avatar Apr 24 '24 09:04 asyncapi-bot

@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 useSpec and useConfig hooks (in my perspective) is not allowed - even if we will use default values for React context.

Next thing, I added (in my comment https://github.com/asyncapi/asyncapi-react/issues/819#issuecomment-1867433195) parent prop to the custom extension component not in vain. You can have extension with this same name in two different places, e.g. channel and operation objects and you should be able to write logic base on that info, if you operate on operation extension or channel extension or another one.

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:

  • please add that parent prop (you can pass item prop from ExtensionsProps)

  • render all extensions with custom components before all extensions without them, you can even put all rest extensions to single Schema and render it as:

    const schema = SchemaHelpers.jsonToSchema(restExtensions);
    return (
      schema && (
        <div className="mt-2">
          <Schema schemaName={name} schema={schema} onlyTitle={true} />
        </div>
      )
    );
    

    so you will end with less code.

If you will have a problems, lets us know!

magicmatatjahu avatar Apr 24 '24 12:04 magicmatatjahu

@magicmatatjahu

render all extensions with custom components before all extensions without them, you can even put all rest extensions to single Schema and render it as:

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:

  • x-sap-odm-entity-name (for this field we plan to have a custom component with a link to elsewhere from the field value)
  • x-sap-odm-oid (no need for a custom component)

Expected behaviour: both fields are rendered close to each other in UI.

pavelkornev avatar Apr 25 '24 14:04 pavelkornev

@pavelkornev HI! Sorry for delay!

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.

Ok, I understand :)

magicmatatjahu avatar Apr 29 '24 08:04 magicmatatjahu

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

@derberg I will add the docs, test and playground showcase.

Regarding your second comment just to clarify. If I understood you correctly, you want to add in library/src/config/default.ts the following:

 ...
  extensions: {
    'x-x': DefaultExtensionComponent,
    'x-linkedin': DefaultExtensionComponent,
  },

Having something like library/src/components/supportedExtensionComponents/DefaultExtensionComponent.tsx with the content:

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 avatar Apr 30 '24 07:04 ductaily

@ductaily not default component, but specific component for each extension, just like you will have probably some specific component for x-sap-odm-oid

so in case of x-x there is a component that renders X logo that is a clickable link pointing to twitter. Same with LinkedIn.

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 avatar Apr 30 '24 08:04 derberg

@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.

pavelkornev avatar May 02 '24 10:05 pavelkornev

@derberg @magicmatatjahu After the merge from the master branch I encountered the issue that my local changes are not displayed when running the UI locally. Assuming this is coming from the major version updates of the react dependencies.

I already ran npm run clean and npm run install and afterwards npm run start. Can you confirm if that is an issue? If not, could you please guide me how can I resolve it to continue the development?

I am running the project on MacOS Version 14.4.1.

ductaily avatar May 02 '24 17:05 ductaily

@ductaily what node version are you using?

@kennethaasan can you have a look?

derberg avatar May 06 '24 09:05 derberg

@ductaily what node version are you using?

@kennethaasan can you have a look?

I am using node version 20.11.0

ductaily avatar May 06 '24 09:05 ductaily

I did

npm run clean
npm install
npm run start

on node 20 on master and playground renders well

when I switch to your branch, installation fails witj

npm ERR! code 1
npm ERR! path /Users/wookiee/sources/asyncapi-react/library
npm ERR! command failed
npm ERR! command sh -c npm run build:dev
npm ERR! > @asyncapi/[email protected] build:dev
npm ERR! > npm run build:esm && npm run build:types && npm run build:styles
npm ERR! 
npm ERR! 
npm ERR! > @asyncapi/[email protected] build:esm
npm ERR! > tsc -p tsconfig.esm.json
npm ERR! 
npm ERR! src/components/Extensions.tsx(76,11): error TS2304: Cannot find name 'reverse'.
npm ERR! npm ERR! Lifecycle script `build:esm` failed with error: 
npm ERR! npm ERR! Error: command failed 
npm ERR! npm ERR!   in workspace: @asyncapi/[email protected] 
npm ERR! npm ERR!   at location: /Users/wookiee/sources/asyncapi-react/library 
npm ERR! npm ERR! Lifecycle script `build:dev` failed with error: 
npm ERR! npm ERR! Error: command failed 
npm ERR! npm ERR!   in workspace: @asyncapi/[email protected] 
npm ERR! npm ERR!   at location: /Users/wookiee/sources/asyncapi-react/library

npm ERR! A complete log of this run can be found in: /Users/wookiee/.npm/_logs/2024-05-13T10_18_29_742Z-debug-0.log

derberg avatar May 13 '24 10:05 derberg

Hi @derberg ,

Thank you for checking. I updated the Extensions.tsx. You should be able to install it now.

ductaily avatar May 13 '24 11:05 ductaily

Still need me to check? I've been offline on holiday so have not seen this until now

kennethaasan avatar May 13 '24 11:05 kennethaasan

@ductaily after your recent update, installation works, and playground renders as expected - not sure what should I do to replicate your issue

derberg avatar May 13 '24 11:05 derberg

Still need me to check? I've been offline on holiday so have not seen this until now

@ductaily after your recent update, installation works, and playground renders as expected - not sure what should I do to replicate your issue

Hi @derberg, @kennethaasan, sorry maybe my explanation was lacking.

Could you please check the following:

  1. do local changes in one of the /library/**/*.tsx
  2. run start Do you see those local changes rendered? The development environment does not recognize my changes.

ductaily avatar May 13 '24 11:05 ductaily

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 playground/package.json file to be "@asyncapi/react-component": "file:../library" for npm to link it correctly on install. But I can't do that because there's a release workflow which changes this version. @derberg, perhaps we can get rid of the bump-react-comp.sh scripts and instead let npm resolve the version. Please see https://www.linkedin.com/pulse/things-i-wish-had-known-when-started-javascript-monorepo-gorej/. This should simplify things quite a bit in the repo.

kennethaasan avatar May 16 '24 12:05 kennethaasan

@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:

  • remove this step https://github.com/asyncapi/asyncapi-react/blob/master/.github/workflows/release-wc-and-playground.yml#L79-L81
  • remove npm run install:reactcomp:playground from https://github.com/asyncapi/asyncapi-react/blob/master/package.json#L41
  • remove this script https://github.com/asyncapi/asyncapi-react/blob/master/package.json#L42
  • remove https://github.com/asyncapi/asyncapi-react/blob/master/playground/package.json#L8
  • remove that file https://github.com/asyncapi/asyncapi-react/blob/master/playground/bump-react-comp.sh
  • and make a change in package.json, about file reference that @kennethaasan described in his comment

derberg avatar May 16 '24 14:05 derberg

haha, very small world. And thanks for fixing this issue!

kennethaasan avatar May 22 '24 13:05 kennethaasan

@derberg @kennethaasan Thanks a lot for your help :)

  1. I added an example for the x-x extension to render a clickable logo. @derberg is this how you imagined it? (See library/src/config/default.ts and library/src/components/supportedExtensions/XExtension.tsx)
  2. I also added documentation to docs/configuration/config-modification.md. Is there any other place where I should add it?
  3. I am unsure how the changes for the Playground showcases should look like. Could you please guide me here?

ductaily avatar May 22 '24 14:05 ductaily

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

I am unsure how the changes for the Playground showcases should look like. Could you please guide me here

just add example x-x: AsyncAPISpec to https://github.com/asyncapi/asyncapi-react/blob/master/playground/specs/streetlights.ts so when playground is loaded, the example is showcased there and visible

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

derberg avatar Jul 25 '24 17:07 derberg

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

I am unsure how the changes for the Playground showcases should look like. Could you please guide me here

just add example x-x: AsyncAPISpec to https://github.com/asyncapi/asyncapi-react/blob/master/playground/specs/streetlights.ts so when playground is loaded, the example is showcased there and visible

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 :) I added an example:

image

ductaily avatar Jul 31 '24 13:07 ductaily

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?

pavelkornev avatar Sep 02 '24 10:09 pavelkornev