open-wc / custom-elements-manifest

Custom Elements Manifest is a file format that describes custom elements in your project.
https://custom-elements-manifest.open-wc.org/
225 stars 37 forks source link

Add unit tests for analyzer (starting with handleJsDoc) #170

Open tlouisse opened 2 years ago

tlouisse commented 2 years ago

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

netlify[bot] commented 2 years ago

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.

thepassle commented 2 years ago

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

tlouisse commented 2 years ago

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