[lit-labs/virtualizer] add keyFunction docs and update jsdocs #4692
Conversation
This provides documentation in the readme for the keyFunction property. It also adds jsdocs for other properties to provide some inline support in editors when using the class reference. By implementing the directive's configuration interface, the element inherits the JSDocs from it as well. Helping us reduce duplication across the code-base. Also apply an `@deprecated` tag to `scrollToIndex` so it is exposed as deprecated.
🦋 Changeset detectedLatest commit: b52eb63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/labs/virtualizer/README.md
Outdated
| ### `keyFunction` property | ||
|
|
||
| Type: `(item: T, index: number) => unknown` | ||
| Generic type name: `KeyFn` from `lit/directives/repeat.js` |
There was a problem hiding this comment.
What does "Generic type name" mean?
There was a problem hiding this comment.
Something I put in here to essentially relay what the type is (and location) so anyone who is wrapping or making a translation layer knows what to pull. However, probably a bad idea in hindsight. It's just clutter to average end users and anyone can follow the path through TS.
I'll drop these in my next update.
packages/labs/virtualizer/README.md
Outdated
| Type: `(item: T, index: number) => unknown` | ||
| Generic type name: `KeyFn` from `lit/directives/repeat.js` | ||
|
|
||
| A function given to the `repeat` directive. This is given the item value and it should return a guaranteed unique key. The unique key helps the repeat directive optimize rendering of large lists. |
There was a problem hiding this comment.
This is the first time repeat is used in the README. It that necessary? I think it'd be better to just describe what the key function does when it's present - it reuses the DOM previously generated for that key.
There was a problem hiding this comment.
The presence of this is driven by lit/lit.dev#1322 . And yes, we can tweak the ending bit. I wrote it in the middle of an other things with the JSDoc stuff, so didn't give it enough time the other day to word it better. Left what it does pretty generic at just "optimize rendering...".
There was a problem hiding this comment.
Also, I described the input and return value purely since some users of the virtualizer may not have used (or remember) the repeat directive usage. Maybe this could actually just link out to those docs. That way there is no duplication of the source content.
There was a problem hiding this comment.
My point about mentioning repeat is that this is a somewhat out of the blue reference to it. Nowhere else do we say that virtualizer uses repeat, and it's entirely an implementation detail. virtualizer could in the future use it's own DOM movement utility.
So I'd rather not mention repeat at all and just document what the key function does from the perspective of virtualizer's behavior.
There was a problem hiding this comment.
Ah ok. I get what you mean now. I'll work on updating the text in a little bit to do that. I think I was trying to not "re-invent the wheel". But yea, it isn't necessarily a detail that consumers of the virtualizer need to know about.
This provides documentation in the readme for the keyFunction property.
It also adds jsdocs for other properties to provide some inline support in editors when using the class reference. By implementing the directive's configuration interface, the element inherits the JSDocs from it as well. Helping us reduce duplication across the code-base.
Also apply an
@deprecatedtag toscrollToIndexso it is exposed as deprecated.Fixes: lit/lit.dev#1322