Attrs initialization without `@attr` decorator

[kristoforsalmin]

Hi there 😄

After upgrading from v1.1.4 to v1.3.2, I noticed that my attributes were no longer shadowed by initializeAttrs(). I started digging and stumbled upon this piece of logic introduced in #191:

https://github.com/github/catalyst/blob/a8fb3ba59a4ac258d0fe64a50ecd3b6997f5f016/src/attr.ts#L39

It prevents initializeAttrs() from doing anything the second time around, thus skipping the manual initialization of attributes. Also, it seems like this change was one of the main things addressed by the above-mentioned PR.

So, what would be the correct way now to initialize the attributes without the @attr decorator?

Thanks!

---

Example

<script>
  class InfoMessage extends HTMLElement {
    open = true

    connectedCallback() {
      initializeAttrs(this, ['open'])
    }
  }

  controller(InfoMessage)
</script>

<info-message data-open="false"></info-message>

1. controller(InfoMessage) wraps original connectedCallback() and calls:

https://github.com/github/catalyst/blob/a8fb3ba59a4ac258d0fe64a50ecd3b6997f5f016/src/core.ts#L14

2. InfoMessage is marked as initialized (attrs.ts):

https://github.com/github/catalyst/blob/a8fb3ba59a4ac258d0fe64a50ecd3b6997f5f016/src/attr.ts#L40

3. The original connectedCallback() is executed, but initializeAttrs() would hit that early-return condition.
4. this.open is stuck in its default state (true) 😔

[keithamus]

In the guide we mention calling defineObservedAttributes. I notice your example doesn't do that. Does your example work when that is called?

[kristoforsalmin]

Hey @keithamus!

Yes, you're right. I'm not calling defineObservedAttributes() as in the example. There are 2 things:

1. initializeAttrs() used to work without calling defineObservedAttributes() in v1.1.4. My use case was that a component has some properties, but they needed no observation. It was OK to just initialize them, so that this.open returns a value form data-open, or a default value defined in the component's class itself open = true. I knew it's a risk to utilize just one part and not the other, but initializeAttrs() was doing the exact thing it promised: taking the values from the DOM and defining them on an instance (shadowing the default values along the way).
2. Unfortunately, calling defineObservedAttributes() won't fix it (please see #208 for the details). When I look at the code of the latest version of Catalyst, I see no "connection" between initializeAttrs() and defineObservedAttributes(). The link is the attr() function...

I created those 2 issues hoping we could tackle them one by one, but now it seems to me they're more interdependent than I thought. Sorry about that 😅

Maybe this summary will make it easier:

attrs.js

// Using a word "instance" everywhere for simplicity

/**
 * Maps instances to their attributes
 */
const attrs = new WeakMap(...)

/**
 * Adds a new attribute to the mapping
 */
function attr(proto, key) {
    // The ONLY place where `attrs` is populated
}

/**
 * Defines computed properties on an instance with values from DOM
 */
function initializeAttrs(instance, names = []) {
    // Takes attribute names from the `names` argument OR from the `attrs` variable
}

/**
 * Defines a static computed `observedAttributes` property on an instance
 */
function defineObservedAttributes(classObject) {
    // Takes attribute names ONLY from the `attrs` variable
}

Please let me know if it's clear or not. I can also prepare an example repo or something 🙂

[keithamus]

Thanks for the detailed reply @kristoforsalmin! We can work to fix the breakage, such that existing code continues to work (after all that’s the semver contract!).

We’re working on a 2.0 release which significantly changes how these fields are managed, especially for non-decorator code. It might be preferable to wait for this release - we hope to get a pre-release out this week. In 2.0, instead of using the decorator you can do the following:

import {controller, attr} from '@github/catalyst'
class AnExampleElement extends HTMLElement {
  static [attr.static] = ['open']
}
AnExampleElement = controller(AnExampleElement)

[kristoforsalmin]

@keithamus cool! I'll just stick to whatever works now and wait for the upcoming release then 👍

Feel free to close those 2 issues (or keep them, if it makes sense). My main goal was to simply let you know about the behavior and my findings. Majority of people are probably using TypeScript anyway, so they won't even notice 😄

And thanks for making Catalyst! So far I only enjoyed its simple design and determination to fix "boilerplaty" aspects of Web Components with just a pinch of features on top and no more than that.