fix(ssr): add more LightningElement methods @W-16867451 #4589
Conversation
| accessKey?: string; | ||
| dir?: string; | ||
| draggable?: boolean; | ||
| hidden?: boolean; | ||
| id?: string; | ||
| lang?: string; | ||
| shadowRoot?: ShadowRoot | null; | ||
| spellcheck?: boolean; | ||
| tabIndex?: number; | ||
| title?: string; |
There was a problem hiding this comment.
Aside from shadowRoot, these are all global attributes. Should they be implemented as getters/setters that just reflect onto __attrs?
There was a problem hiding this comment.
Yes, we'll need to implement proper property reflection. This is already handled in @lwc/engine-server because it gets it from base-lightning-element.ts in @lwc/engine-core. We should also add tests for it if they don't exist.
| @@ -210,74 +181,116 @@ export class LightningElement implements PropsAvailableAtConstruction { | |||
| this.__attrs[attrName] = value; | |||
| } | |||
|
|
|||
| hasAttribute(attrName: string): boolean { | |||
| return Boolean(this.__attrs && attrName in this.__attrs); | |||
There was a problem hiding this comment.
| return Boolean(this.__attrs && attrName in this.__attrs); | |
| return Boolean(this.__attrs && attrName in this.__attrs && this.__attrs[attrName] !== null); |
Technically you can setAttribute('foo', null) and it's the same as removing it.
There was a problem hiding this comment.
Opted to go the other way -- setAttribute('foo', null) will now remove the attribute from __attrs.
There was a problem hiding this comment.
Whoops I was wrong on this! getAttribute returns null but setAttribute sets to string "null". 😆
| accessKey?: string; | ||
| dir?: string; | ||
| draggable?: boolean; | ||
| hidden?: boolean; | ||
| id?: string; | ||
| lang?: string; | ||
| shadowRoot?: ShadowRoot | null; | ||
| spellcheck?: boolean; | ||
| tabIndex?: number; | ||
| title?: string; |
There was a problem hiding this comment.
Yes, we'll need to implement proper property reflection. This is already handled in @lwc/engine-server because it gets it from base-lightning-element.ts in @lwc/engine-core. We should also add tests for it if they don't exist.
Details
Made the following changes to
@lwc/ssr-runtimeto better match@lwc/engine-server:addEventListenerandremoveEventListeneras no-opshasAttributebecause we missed that last time@lwc/engine-serverare left as-isDoes this pull request introduce a breaking change?
Does this pull request introduce an observable change?
GUS work item
W-16867451