github / catalyst

Catalyst is a set of patterns and techniques for developing components within a complex application.
https://github.github.io/catalyst/
MIT License
1.32k stars 49 forks source link

Setting an @attr property in a click listener not reflected in getter until next run loop (v2.0.0-beta) #293

Closed ryecroft closed 1 year ago

ryecroft commented 1 year ago

I'm confused by the behaviour of properties managed by the @attr decorator during click listener callbacks. Setting one of these properties during the connectedCallback() is immediately reflected when calling the getter for it, but this is not the case in click listeners.

Here's my component definition:

import { controller, attr } from '@github/catalyst'

@controller
class TestComponentElement extends HTMLElement {

  @attr testProp: string

  connectedCallback() {
    this.innerHTML = `hello`
    this.setAttribute( 'data-action', 'click:test-component#wasClicked' )
    console.log( this.testProp )  // prints 'undefined' as expected
    this.testProp = 'A'
    console.log( this.testProp ) // prints 'A' as expected
    this.testProp = 'B'
    console.log( this.testProp ) // prints 'B' as expected
  }

  // this is called when clicking the element
  wasClicked( _evt ) {
    console.log( this.testProp ) // prints 'B' as expected (set to this at end of connectedCallback())
    this.testProp = 'C'
    console.log( this.testProp ) // prints 'B' - not expected
    this.testProp = 'D'
    console.log( this.testProp ) // prints 'B' - not expected
    setTimeout( () => {
      console.log( this.testProp ) // prints 'D'
    }, 0 )
  }

}

export default TestComponentElement

the html is like this:

<!DOCTYPE html>
<html lang='en'>
    <head>
        <meta charset='UTF-8' />
    </head>
    <body>
        <div style='min-height: 20pt;'></div>
        <test-component></test-component>
    </div>
    <script type='module' src='/src/main.ts'></script>
</html>

I see here that the injected setter for the @attr decorator is async, and that it's not awaited here

Removing that async and the await Promise.resolve() here makes this unexpected behaviour go away (but breaks a load of other stuff in a real project).

Is this behaviour expected? Thanks

ryecroft commented 1 year ago

Closed this issue because I think there's something weird with my setup. I now cannot reproduce it =\

koddsson commented 1 year ago

Closed this issue because I think there's something weird with my setup. I now cannot reproduce it =\

Probably! Me and @keithamus looked at this and couldn't reproduce either. We added a test to validate in https://github.com/github/catalyst/commit/a874af99cf3c0bd0321bef4ea6b500a3b7a11963. Feel free to re-open with more info if you get it.

Thanks for the bug report, hopefully you get this sorted in your app!