web-component-analyzer icon indicating copy to clipboard operation
web-component-analyzer copied to clipboard

support @attr @readonly

Open bennypowers opened this issue 4 years ago • 1 comments

This issue follows up on https://github.com/runem/web-component-analyzer/issues/125#issuecomment-562213898

Input

/**
 * Whether or not the element has an error
 * @attr error
 * @readonly
 */
get error() { return this.#error }
set error(_) {}

static get properties() { return { error: { reflect: true } } }

onError(error) {
  const oldError = this.error;
  this.#error = error
  this.requestUpdate('error', oldError)
}

bennypowers avatar Dec 16 '19 12:12 bennypowers

This is an issue with the following line responsible for merging modifiers: https://github.com/runem/web-component-analyzer/blob/master/src/analyze/stages/merge/merge-util.ts#L37

Here's a short description of what happens and why I'm still considering what would be the expected behavior.

When analyzing, it finds the member error three times.

  1. get error ==> will have the readonly modifier because it's a getter (and because of the @readonly jsdoc as well)
  2. set error
  3. properties() { return { error ... } }

These are merged afterwards. As the readonly modifier is only to be found from get error but not from the others, the readonly modifier is stripped. If changing && to || on line 37 it will work in your case and output the readonly modifier.

However, the reason for the current behavior is that generally merging a getter with a setter should not result in the readonly modifier:

get myProp() { }
set myProp(_) {}

I think the solution to your problem is that readonly should always be force-applied to the merged result when explicitly using the jsdoc @readonly modifier :-)

runem avatar Dec 18 '19 13:12 runem