custom-elements-manifest icon indicating copy to clipboard operation
custom-elements-manifest copied to clipboard

Add unit tests for analyzer (starting with handleJsDoc)

Open tlouisse opened this issue 3 years ago • 3 comments

Hi,

When running the tool on our Lion components (LionInputAmount) in particular, we found out that handleJsDoc does not recognize object params built from multiple @param tags.

For instance:

class X extends HTMLElement {
  /**
   * @param {object} opts
   * @param {string} [opts.currency]
   */
  callMe({currency}) {
    return currency + '10.00';
  }
}

Is expected to return the following parameters:

     [
        {
          name: 'opts',
          type: {
            text: '{currency?: string}',
          },
        },
      ]

(currently it returns two params).

I created a failing unit test for the scenario above (it's skipped on purpose now, so this can be merged if wanted).

@thepassle since there were only integration tests for the analyzer package so far, feel free to say if you don't agree with the approach or want to do this in a different way :)

tlouisse avatar May 04 '22 07:05 tlouisse

Deploy Preview for custom-elements-manifest-analyzer ready!

Name Link
Latest commit 181826d71b221db1bcc028efb88d2fe04b383825
Latest deploy log https://app.netlify.com/sites/custom-elements-manifest-analyzer/deploys/62722e72067d3d00089ce7ae
Deploy Preview https://deploy-preview-170--custom-elements-manifest-analyzer.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] avatar May 04 '22 07:05 netlify[bot]

Did you also want to actually apply the function anywhere in this PR? Or are you planning that for a separate PR?

thepassle avatar May 04 '22 11:05 thepassle

Did you also want to actually apply the function anywhere in this PR? Or are you planning that for a separate PR?

We can work around it atm, so I added .skip for now. But maybe in the future I want to do it in a separate PR :). @dakmor and me just had a look, and it should be possible to get all type info from the TS AST.

Screenshot 2022-05-04 at 16 28 58 (Type info is found on the right)

See: https://ts-ast-viewer.com/#code/MYGwhgzhAEAa0FMAeAXBA7AJjAEgFQFkAZAURAQFsMVoBvAKGmgHoAqVxp16AAQAcwAJzAU6AewBGAKwTAUAX2hi+KCJ2jd+QkXQgpBAS3QBzRQG1lqgHTAAroMEZgATwC661s07AwIEAQQAClo7BydneQBKOnVoRxR7dGhQx3QXaABqaAByAEYABit8-OyAbk55enkgA

tlouisse avatar May 04 '22 14:05 tlouisse