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

feat: add configuration to always show message examples

Open tsauerwein opened this issue 1 year ago • 16 comments

Description

Changes proposed in this pull request:

  • Adds a configuration show.messageExamples, which when true also shows examples for the messages in the Messages section (and not only for the messages in the "Operations" section).

Background:

Maybe we are using the AsyncAPI CLI not in the intended way. But we have messages that are not referenced in one of the operations, so that the examples for these messages are not visible. This PR is proposing to add a new configuration, which will show examples for the messages listed in the "Messages" section.

Related issue(s)

tsauerwein avatar Jun 17 '24 14:06 tsauerwein

@tsauerwein hey, thanks for PR. Can you clarify one thing? so normally examples are always visible, except of this single case with operation? also you mean they are collapsed by default, or not visible at all?

with your change, did you try to build this project locally and check in local playground if intended change took place?

derberg avatar Jun 20 '24 11:06 derberg

@derberg Thanks for taking the time to reply! Currently, the examples for messages are only shown when the messages are displayed for an operation. If you have a message that is not used in one of the operations, the message is listed in the "Messages" block, but the example for the message is not visible.

Eg. it looks like this: Screenshot 2024-06-20 at 14 24 52

With this new configuration, the example for the message would be visible:

Screenshot 2024-06-20 at 14 20 50

This screenshot was taken from the playground running locally. Also, we have built the library, and are using library/browser/standalone/without-parser.js for our documentation.

I hope this makes it a bit clearer. If not, let me know. :)

tsauerwein avatar Jun 20 '24 12:06 tsauerwein

@tsauerwein ah that's what you mean. Sorry, I misunderstood the initial message.

We need I think a better name for messageExamplesAlways and additional description in docs. Hard to pick a better one, but current one can be confusing as might refer to examples in general. How about we call it just messageExamples only and in docs you provide a detailed comment what that is?

derberg avatar Jun 20 '24 13:06 derberg

side comment, what is the use case for having one message visible in docs, that is not associated with any channel and operation

also, can you share how did you make library/browser/standalone/without-parser.js work for you, with maybe example as some users struggle with that a bit -> https://github.com/asyncapi/asyncapi-react/issues/956

derberg avatar Jun 20 '24 13:06 derberg

@tsauerwein ah that's what you mean. Sorry, I misunderstood the initial message.

We need I think a better name for messageExamplesAlways and additional description in docs. Hard to pick a better one, but current one can be confusing as might refer to examples in general. How about we call it just messageExamples only and in docs you provide a detailed comment what that is?

Sure, I renamed it to messageExamples and added the following to the docs:

The examples for messages shown within an operation are always displayed. To also show examples for the standalone messages in the "Messages" section, set messageExamples to true.

Let me know if that is clear enough.

side comment, what is the use case for having one message visible in docs, that is not associated with any channel and operation

We might be abusing AsyncAPI CLI a bit here. But we have a MQTT topic configuration/{configuration} which is published for each configuration. And we want to document the schema for each configuration. We could also list each configuration message in the operation. But then we don't have a nice table-of-content in the sidebar, where you could directly jump to the messages/configuration.

tsauerwein avatar Jun 24 '24 12:06 tsauerwein

also, can you share how did you make library/browser/standalone/without-parser.js work for you, with maybe example as some users struggle with that a bit -> #956

We've built it with npm run build:standalone inside the library directory. (Node v18.20.3)

But npm run build in the project root fails with Module not found: Error: Can't resolve 'react' in ... like in this Action: https://github.com/asyncapi/asyncapi-react/actions/runs/9496432356/job/26170818977

tsauerwein avatar Jun 24 '24 12:06 tsauerwein

We might be abusing AsyncAPI CLI a bit here. But we have a MQTT topic configuration/{configuration} which is published for each configuration. And we want to document the schema for each configuration. We could also list each configuration message in the operation. But then we don't have a nice table-of-content in the sidebar, where you could directly jump to the messages/configuration.

can you please clarify. So because you want to see a message in side bar navigation, you decided to provide message definition only in components, instead under the channel payload?

derberg avatar Jun 24 '24 13:06 derberg

@tsauerwein please also look into linter errors

btw, thanks for reporting web component release is failing -> https://github.com/asyncapi/asyncapi-react/pull/1019

derberg avatar Jun 24 '24 15:06 derberg

can you please clarify. So because you want to see a message in side bar navigation, you decided to provide message definition only in components, instead under the channel payload?

Basically, yes. We are using it to document schemas for configurations. So, it means there is one channel/operation and many messages (100+). If we would add each configuration message to the the operation, the message schema would be shown twice, within the operation and then again in the "Messages" sections. Also, the message links in the sidebar take you to the messages in the "Messages" section, where you don't have the example (without the proposed change). And the table-of-content for the messages is quite useful considering the big number of configurations.

To give you an idea, this is what the document structure looks like: configurations.yml.txt

I would totally understand, if you say that we are using AsyncAPI not in the intended way. But we are using AsyncAPI to document our MQTT topics and like the tooling and the rendered documentation. That's why we also wanted to use AsyncAPI to document the configuration schemas for a single operation.

tsauerwein avatar Jun 25 '24 08:06 tsauerwein

release fixed: https://github.com/asyncapi/asyncapi-react/actions/runs/9661367651

yeah the problem with how you use it is that messages are not linked with channel and channel specifies only one message, without defining a payload, which means any payload is accepted. It might work for you short term, but what if you'd like to use other AsyncAPI tools - these won't understand the workaround.

proper way would be:

channels:
  configuration:
    address: configuration/{configuration}
    messages:
      configuration_A:
        $ref: '#/components/messages/CONFIG_A'
      configuration_B:
        $ref: '#/components/messages/CONFIG_B'

If we would add each configuration message to the the operation, the message schema would be shown twice, within the operation and then again in the "Messages" sections. Also, the message links in the sidebar take you to the messages in the "Messages" section

I recommend long term focus on using AsyncAPI correct way and just contribute to this component solution that will make side bar messages navigation work in the way, that if Messages section is disabled, messages in navigation point to messages in operations. Although, because same message can be part of many different operations, probably better would be to add support for a different navigation, dedicated message navigation-list as sub-nav under each operation.

Anyway, just sharing an opinion. Not bothering you anymore and playing a smart ass 😄

derberg avatar Jun 25 '24 11:06 derberg

@derberg Thanks for fixing the release! And thanks for the feedback, you are absolutely right.

The "Test NodeJS PR - ubuntu-latest" job has failed. Could it be that package.json and package-lock.json are out-of-sync on master? https://github.com/asyncapi/asyncapi-react/actions/runs/9661830217/job/26650463755?pr=1016

tsauerwein avatar Jun 25 '24 12:06 tsauerwein

Working on it. For some reason bump of react component in web component did not work as expected.

derberg avatar Jun 25 '24 12:06 derberg

fix is up https://github.com/asyncapi/asyncapi-react/pull/1026

derberg avatar Jul 02 '24 18:07 derberg

fix is up #1026

Thanks! I will rebase as soon as the fix is merged.

tsauerwein avatar Jul 03 '24 05:07 tsauerwein

/rtm

derberg avatar Jul 03 '24 18:07 derberg

:tada: This PR is included in version 2.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

asyncapi-bot avatar Jul 03 '24 18:07 asyncapi-bot

Thanks for the review and making the release, @derberg! 🚀

tsauerwein avatar Jul 04 '24 06:07 tsauerwein