bug: varying behavior with reflect, undefined, null, empty string values

[odoe]

Prerequisites

• [X] I have read the Contributing Guidelines.
• [X] I agree to follow the Code of Conduct.
• [X] I have searched for existing issues that already report this problem, without success.

Stencil Version

4.8.0

Current Behavior

When a prop uses reflect, it has different behavior if value is set to undefined, null or "".

• "" - sets attribute to ""
• undefined - leaves attribute as-is
• null - removes attribute from element

Expected Behavior

I'm not sure what the expected behavior should be. I think in all three cases, the attribute should be removed, but have concerns about the undefined -> null loop warning popping up again. Either way, the behavior should probably be consistent.

System Info

Tested in:
FireFox: 119.0.1
Chrome: 119.0.6045.159 

// npx stencil info    
  System: node 20.2.0
    Platform: darwin (23.1.0)
   CPU Model: Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz (12 cpus)
    Compiler: /Users/rene8209/Documents/dev/test-apps/web-comps-testing/my-comp/node_modules/@stencil/core/compiler/stencil.js
       Build: 1701098446
     Stencil: 4.8.0 🌞
  TypeScript: 5.2.2
      Rollup: 2.42.3
      Parse5: 7.1.2
      Sizzle: 2.42.3
      Terser: 5.24.0

Steps to Reproduce

Sample component:

@Component({
  tag: 'my-component',
  styleUrl: 'my-component.css',
  shadow: false,
})
export class MyComponent {
  ...
  @Prop({ reflect: true }) myValue = 'My Value';

  @Watch('myValue')
  myValueChangeHandler(newValue: string, oldValue: string) {
    console.log('myValueChangeHandler', newValue, oldValue);
  }
  ...
  render() {
    const myValue = this.myValue ? this.myValue : 'default';
    return (
      <div>
        Hello, World! I'm {this.getText()} - {myValue}
      </div>
    );
  }
}

In application:

  <my-component my-value="hello" first="Stencil" last="'Don't call me a framework' JS"></my-component>
  <script>
    const myComp = document.querySelector('my-component');
    setTimeout(() => {
      console.log('set value');
      myComp.myValue = ''; // sets attribute to ""
      // myComp.myValue = undefined; // leaves attribute as-is
      // myComp.myValue = null; // removes attribute
    }, 5000);
  </script>

Code Reproduction URL

https://github.com/odoe/stencil4-component-undefined-null-empty-string-issue/tree/main

Additional Information

This is similar to #3586 Funny enough, this fixes #2814, at least for us, because in 2x, if a user set a value to undefined, it would immediately be converted to null and cause that console warning. We could handle it under the hood to prevent render loops, but was nice to see it fixed.

[rwaskiewicz]

Thanks! I was able to confirm using your repro case that this is indeed a bug.

The team is aware of issues similar to this one + general rough edges in some use cases when using @Prop() when using null and undefined. I'm going to add this to the grouping of issues we have in our internal backlog.

We believe that although this is a bug that we'd be 'fixing', there is a decent possibility a fix would be a breaking change. We are tentatively scheduling work on fixing some of these issues (including this one) for Stencil v5 to avoid negatively impacting folks.