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

fix: update jsdocs for listbox #774

Merged
merged 2 commits into from
Mar 2, 2022

Conversation

streamside
Copy link
Contributor

Motivation

Update JSDoc to improve API specification and typescript definitions. Changes:

  • Add types for ListBox options containing fixed set of valid configs. For example, FrequencyMode and Direction.
  • Use @interface to describe objects with a number of properties (instead of @typedef)

@streamside streamside changed the title fix: update types in jsdocs fix: update jsdocs for listbox Mar 2, 2022
Copy link
Collaborator

@T-Wizard T-Wizard left a comment

Choose a reason for hiding this comment

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

I am OK with it but,
the change to @interface from @typedef should not be needed.
It is used a lot in picasso.js and if generating type from @typedef is broken we should look into it.

@streamside
Copy link
Contributor Author

I am OK with it but, the change to @interface from @typedef should not be needed. It is used a lot in picasso.js and if generating type from @typedef is broken we should look into it.

I agree with you Tobias. When having another look at this Picasso.js uses scriptappy-from-jsdoc to create the API specification while Nebula.js uses @scriptappy/from-jsdoc. They handle @typedef differently and due to this the typescript defs becomes broken.

Long term I think we should use @scriptappy/from-jsdoc in all repos which means that this problem needs to be resolve in there or adapt @scriptappy/to-dts to support the format created by @scriptappy/from-jsdoc.

Example from Picasso.js (simplified):

  /**
   * @typedef {object} Registries
   * @property {registry} component Component registry
   * @property {registry} data Data registry
   * @property {registry} formatter Formatter registry
   * @property {registry} interaction Interaction registry
   * @property {registry} renderer Renderer registry
   * @property {registry} scale Scale registry
   */

  // API Specification
  {
    "Registries": {
      "kind": "object",
      "entries": {
        "component": {
          "description": "Component registry",
          "type": "#/definitions/registry"
        },
        "data": {
          "description": "Data registry",
          "type": "#/definitions/registry"
        },
        "formatter": {
          "description": "Formatter registry",
          "type": "#/definitions/registry"
        },
        ...
    }

Example from Nebula.js:

/**
 * @typedef {object} Segment
 * @property {string} segment The sub-string/segment cut out from the original label.
 * @property {boolean} highlighted A flag which tells whether the segment should be highlighted or not.
 */

// API Specification
{
    "Segment": {
      "kind": "alias",  // <-- object is wrapped in an alias
      "items": {
        "kind": "object",
        "entries": {
          "segment": {
            "description": "The sub-string/segment cut out from the original label.",
            "type": "string"
          },
          "highlighted": {
            "description": "A flag which tells whether the segment should be highlighted or not.",
            "type": "boolean"
          }
        }
      }
    }
}

@veinfors veinfors merged commit c607e37 into qlik-oss:master Mar 2, 2022
@streamside streamside deleted the fix/stardust-docs branch March 2, 2022 10:39
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.

3 participants