[reactive-element] Avoid @query caching null results before the first render

[MaxArt2501]

Should this be an RFC?

• This is not a substantial change

Which package is this a feature request for?

Lit Core (lit / lit-html / lit-element / reactive-element)

Description

If you access @queryd property before the first render, if will yield null but will stay null is you set the cache flag to true. The documentation should probably warn the users that this might happen (might open a separate issue on lit.dev repo).

I think it could be beneficial if @query caches the result only if it actually has a chance to return a non-null result, i.e. only if hasUpdated is true. Something like this:

descriptor.get = function (this: ReactiveElement) {
  // No caching here
  if (!this.hasUpdated) return null;
  // Rest of the getter code...
};

I'm available to open a PR for this.

Alternatives and Workarounds

No alternative that would be as simple as using @query straight away.

One could redefine their own @query selector, but of course it'd need to be kept in line with Lit's decorator.

Alternatively, one could check if hasUpdated is true if they need to access the queried property. A secondary getter could be handy, but adds overhead:

class MyElement extends LitElement {
  @query('button', true) private _button!: HTMLButtonElement | null;

  get button() {
    return this.hasUpdated ? this._button : null;
  }
  // ...
}

[justinfagnani]

I think this sounds like a good idea. I wonder in what cases you're accessing the query field before first render though?

[justinfagnani]

@rictic thoughts?

[rictic]

I feel like I've seen code kinda like:

@query('button', true) button: HTMLButtonElement|undefined;

clickButton() {
  this.button?.click();
}

Is that the best? No. Should it cache null? That does seem worse

[rictic]

It'd add few bytes, little overhead, and is otherwise a solid improvement, so I'm in favor.

Even better with a dev mode warning for accessing a query before first updated.

[MaxArt2501]

@rictic That's exactly the main use case that came to my mind.

The other case is when queried properties are accessed in willUpdate.