Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: improve documentation related to File templates feature #1230

Merged

Conversation

Florence-Njeri
Copy link
Collaborator

Description
Improve documentation related to File templates feature:

  • Separate nunjucks and react render engine file template docs
  • Link a real life example of react render engine file template in use

Related issue(s)
Resolves #1199

@aeworxet
Copy link

@asyncapi/bounty_team

@Florence-Njeri Florence-Njeri changed the title docs: Improve documentation related to File templates feature docs: improve documentation related to File templates feature Jul 23, 2024
@Florence-Njeri Florence-Njeri marked this pull request as ready for review July 23, 2024 07:45
@Florence-Njeri Florence-Njeri added the bounty AsyncAPI Bounty program related label label Jul 25, 2024
Copy link
Collaborator

@Gmin2 Gmin2 left a comment

Choose a reason for hiding this comment

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

LGTM

just one point you can add more examples

@derberg
Copy link
Member

derberg commented Jul 29, 2024

@Gmin2 please suggest one

Copy link

changeset-bot bot commented Jul 29, 2024

⚠️ No Changeset found

Latest commit: 8ef5496

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Florence-Njeri Florence-Njeri self-assigned this Jul 30, 2024
@derberg
Copy link
Member

derberg commented Aug 11, 2024

@Gmin2 kind reminder

Copy link
Collaborator

@Gmin2 Gmin2 left a comment

Choose a reason for hiding this comment

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

There could be an example of generating with varied content based on different components

import { File } from '@asyncapi/generator-react-sdk';

export default function({ asyncapi }) {
  const files = [];

  // Generate files for channels
  asyncapi.channels().forEach((channel, channelName) => {
    files.push(
      <File name={`channels/${channelName}.md`}>
        # Channel: {channelName}

        Description: {channel.description() || 'No description provided'}

        ## Operations
        {channel.publish() && <div>Publish: {channel.publish().summary()}</div>}
        {channel.subscribe() && <div>Subscribe: {channel.subscribe().summary()}</div>}
      </File>
    );
  });
  return files
  }

other then that looks fine

@Florence-Njeri
Copy link
Collaborator Author

Hey @Gmin2 I have updated the PR

Copy link
Collaborator

@Gmin2 Gmin2 left a comment

Choose a reason for hiding this comment

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

LGTM

@derberg
Copy link
Member

derberg commented Aug 19, 2024

folks we need an example that is valid. Provided example is not valid and looks like generated with AI, as it is inventing not existing features and using old API.

it is critical to always run examples manually before they are added to docs - no matter if written by ChatGPT or a human (never trust an engineer). Since we have https://github.com/asyncapi/generator/tree/master/apps/generator/test/test-templates/react-template/template in the generator now, it should be as simple as locally adding new file with an example, then you run the generation with CLI and see the result

here is a simplified version of the example:

import { File, Text } from "@asyncapi/generator-react-sdk";

export default function ({ asyncapi }) {
  const files = [];

  // Generate files for channels
  asyncapi.channels().forEach((channel) => {
    const channelName = channel.id();

    files.push(
      <File name={`${channelName}.md`}>
        <Text newLines={2}># Channel: {channelName}</Text>
        <Text>
          {channel.hasDescription() && `${channel.description()}`}
        </Text>
      </File>
    );
  });
  return files;
}

it shows:

  • how multiline files generation must be handled with Text component, otherwise all gets generated as one line
  • it shows how to work with new API of the parser - more API details in https://github.com/asyncapi/parser-api/blob/master/docs/api.md
  • <File name={channels/${channelName}.md}> will not work, you will get an error. If you want to generate channel MD files in channels directory, you need to put template file in channels directory

@Gmin2 your example is good from idea point of view, so now just please take my sample end extend with part where you render operations information (forget about publish and subscribe, it is not there in new AsyncAPI anymore)

@Florence-Njeri
Copy link
Collaborator Author

Hey @derberg @Gmin2 I have updated the PR with the up-to-date code snippet. Please help me re-review. Thank you!

const schemas = asyncapi.allSchemas();
// schemas is an instance of the Map
return Array.from(schemas).map(([schemaName, schema]) => {
Copy link
Member

Choose a reason for hiding this comment

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

schemas is not an array but a map, so like I suggest in my previous comment to the PR, better use forEach

and yeah, you need to manually test if file is working, as this way you would know something is wrong

during generation I get Generator Error: object is not iterable (cannot read property Symbol(Symbol.iterator))

btw, this example also needs import { File, Text } from "@asyncapi/generator-react-sdk";

@Florence-Njeri
Copy link
Collaborator Author

Florence-Njeri commented Aug 31, 2024

Hey @derberg I fixed the code. Please take another look

Copy link

sonarcloud bot commented Sep 2, 2024

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

I updated schema example as it was invalid. Without checking examples manually, we should not put them in docs as they are not helpful and only confuse

@derberg
Copy link
Member

derberg commented Sep 2, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 5a71b72 into asyncapi:master Sep 2, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty program related label ready-to-merge
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

[📑 Docs]: Improve documentation related to File templates feature
7 participants