lwc icon indicating copy to clipboard operation
lwc copied to clipboard

CustomElementConstructor should accept props that exist before upgrade

Open nolanlawson opened this issue 1 year ago • 8 comments

Description

Pre-existing props on an HTML element, when combined with CustomElementConstructor, do not result in the correct props being set on the LWC component.

<x-app></x-app>
// app.js
export default class extends LightningElement {
  @api 
  foo
}
// main.js
import App from "x/app";

// prop exists before custom element upgrade
document.querySelector('x-app').foo = 'foo'

customElements.define('x-app', App.CustomElementConstructor)

Expected: the prop is correctly set from LWC's perspective. Actual: it is not set, and in any subsequent setter calls are ignored.

If you reverse the order of the last two lines, it the prop is correctly set. (Repro)

Screenshot 2023-10-03 at 8 49 33 AM

Steps to Reproduce

Repro

Related issue

As described in this post, we should handle pre-existing props in the constructor (as Lit does).

nolanlawson avatar May 02 '23 20:05 nolanlawson

A possible solution to this problem is to handle the conversion of attributes to props within the connectedCallback lifecycle hook of your LWC component. This way, when your component is connected to the DOM, it can read the attributes and set the corresponding props.

Here's an example of how you can achieve this in your x/app component:

import { LightningElement, api } from 'lwc';

export default class App extends LightningElement {
    _foo;

    // Create a getter and setter for the foo property in your x/app component:
    @api
    get foo() {
        return this._foo;
    }

    set foo(value) {
        this._foo = value;
    }

    // Implement the connectedCallback lifecycle hook in your x/app component to convert the foo attribute to a prop:
    connectedCallback() {
        if (this.hasAttribute('foo')) {
            this.foo = this.getAttribute('foo');
        }
    }
}

This method allows you to handle any attribute-to-prop conversion within the component itself, providing more control over the behavior.

jayanth-kumar-morem avatar May 06 '23 17:05 jayanth-kumar-morem

@nolanlawson Can I pick this up?

yashpatel1998 avatar Sep 26 '23 17:09 yashpatel1998

@yashpatel1998 Yes, but I think the bug description is actually wrong. Let me fix it.

nolanlawson avatar Sep 26 '23 18:09 nolanlawson

OK, the bug still exists. I'm surprised. I don't know why this is happening, since I thought attributeChangedCallback automatically handled pre-existing attributes.

There is a separate issue where pre-existing props do not get applied (repro), but I think maybe that should be handled in a separate bug.

nolanlawson avatar Sep 26 '23 18:09 nolanlawson

@nolanlawson attributeChangedCallback does handle pre-existing attributes provided they are public and marked as @api. As per my understanding of the issue, we would like to set non public props on a CustomElementConstructor if they are passed as attributes. Am I correct?

yashpatel1998 avatar Oct 03 '23 00:10 yashpatel1998

@yashpatel1998 You're right (repro). I must have forgotten to add @api. 😅

I'll update the bug title and description.

nolanlawson avatar Oct 03 '23 15:10 nolanlawson

As per my understanding of the issue, we would like to set non public props on a CustomElementConstructor if they are passed as attributes. Am I correct?

@yashpatel1998 Sorry, this is not correct. I've updated the bug description with the expected behavior.

It's not about @api – it's about setting a prop on an HTMLElement before customElements.define() is called. Pre-existing props (not attributes) are not handled correctly.

nolanlawson avatar Oct 03 '23 15:10 nolanlawson

@nolanlawson Ah, this description is clear to me now. Although the prop is set before the CustomElementConstructor invocation the compiler does not set it while upgrading it via customElements.define()

yashpatel1998 avatar Oct 03 '23 15:10 yashpatel1998