It would be cool if we could run tests in karma so we can have more confidence in tests since they will be running in the context of a browser that has Custom Elements.

cc/ @muan

At work with @babel/plugin-proposal-decorators, decorators @target and @targets returns undefined. Made small repository where reproduce error https://github.com/onEXHovia/catalyst-decorators

At a quick inspection, if you add return for methods https://github.com/github/catalyst/blob/main/src/target.ts the problem is solved, but not sure if it's correct solution

export function target<K extends string>(proto: Record<K, unknown>, key: K): void {
  meta(proto, 'target').add(key)
  return Object.defineProperty(proto, key, {
    configurable: true,
    get() {
      return findTarget(this, key)
    }
  })
}
.....
.....

I'm following the v2 documentation (https://catalyst.rocks/guide-v2/targets) but the following code snippet doesn't work with JS:

import { actionable, targetable, target } from '@github/catalyst'

export default actionable(targetable(class extends HTMLElement {
  [target.static] = ['fullAddressField'] // on html template there is a data-target="controller-name.fullAddressField" attr
  
  toggleAddressField () {
    this.fullAddressField.required = false
  }
}))

Is this possible to use this feature in JS or it works only via TS?

It would be convenient to know when targets change, so actions can be taken accordingly.

For attributes, I define in the markup data-my-attr-name and in the controller as @attr myAttrName

For targets however, I have to define the target name in both the markup and the controller using camelCase:
data-targets="my-controller.myTargetsName" and @targets myTargetsName
I think it would be nice (more consistent?) if the markup could support kebab-case.

First,define the primary-button component,like this:

@controller
class PrimaryButtonElement extends HTMLElement {
  connectedCallback(){
       const itemTemplate = () => {
           return html`
          <button>
              ???
          </button>
          `
        }

        render(itemTemplate(), this)
   }
}

Then,use the component like this,

<primary-button>new issue</primary-button>

The question is how to render the component's content in the controller?
Thanks!

I was pairing with @koddsson on this but got blocked by #32 when trying to add a test.

  class Boom extends HTMLElement {
    constructor() {
      super()
      const shadow = this.attachShadow({mode: 'open'})
      shadow.innerHTML = `<style>:host { display: block; }</style><slot></slot>`
    }
  }

Currently all elements we have experimented with have a d-block class added to them. We are familiar with custom elements and know that the default is inline, but this won't be true to many others. To them the layout just appear to be broken when they wrap elements inside of the controller element. Since the controller element is supposed to have light DOM, I think it's reasonable to make it a default block.

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

Right now Safari doesn't support is= and so we cannot extend built ins, which is a shame because they include some good features which we could benefit from, for example <dialog is="foo-bar">. We should find a way to compose controllers in a different way.

Problem

In the introduction, there are links to Custom Elements, Actions, and Targets.

All of these links 404 (ex. https://github.github.io/guide/custom-elements)

Expected behavior

These links should go to the correct pages:
https://github.github.io/catalyst/guide/targets/
https://github.github.io/catalyst/guide/actions/

(There doesn't appear to be a Custom Elements page in the docs)

Problem

In the reference docs, there are links to code like: https://github.com/github/catalyst/blob/1026141/catalyst/src/bind.ts#L7

These links 404.

Expected behavior

The links would go to the correct line of code:

When I go to the Guide, there's an index on the left-hand side and each topic also has Previous / Next buttons at the bottom of the page. If you go to the Testing topic, the Next button will take you to Abilities, followed by Providable and then Create Ability. But the index on the side jumps from Testing straight to Lazy Elements.

It seems like the index should be updated to include these additional topics.

I created an npm project with webpack. After adding @github/catalyst and importing anything from it, webpack no longer works and produces the errors presented below.

My Environment

% node -v
v14.15.5

% npx webpack --version
webpack 5.24.0
webpack-cli 4.5.0

https://github.com/heynan0/using-catalyst

The Errors

% npx webpack
assets by status 789 bytes [cached] 1 asset
orphan modules 292 bytes [orphan] 1 module
./src/index.js + 1 modules 547 bytes [built] [code generated]

WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value.
Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/

ERROR in ./node_modules/@github/catalyst/lib/index.js 1:0-45
Module not found: Error: Can't resolve './bind' in '/Users/heynan0/github/using-catalyst/node_modules/@github/catalyst/lib'
Did you mean 'bind.js'?
BREAKING CHANGE: The request './bind' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./src/index.js 1:0-43

ERROR in ./node_modules/@github/catalyst/lib/index.js 2:0-38
Module not found: Error: Can't resolve './register' in '/Users/heynan0/github/using-catalyst/node_modules/@github/catalyst/lib'
Did you mean 'register.js'?
BREAKING CHANGE: The request './register' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./src/index.js 1:0-43

ERROR in ./node_modules/@github/catalyst/lib/index.js 3:0-55
Module not found: Error: Can't resolve './findtarget' in '/Users/heynan0/github/using-catalyst/node_modules/@github/catalyst/lib'
Did you mean 'findtarget.js'?
BREAKING CHANGE: The request './findtarget' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./src/index.js 1:0-43

ERROR in ./node_modules/@github/catalyst/lib/index.js 4:0-43
Module not found: Error: Can't resolve './target' in '/Users/heynan0/github/using-catalyst/node_modules/@github/catalyst/lib'
Did you mean 'target.js'?
BREAKING CHANGE: The request './target' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./src/index.js 1:0-43

ERROR in ./node_modules/@github/catalyst/lib/index.js 5:0-42
Module not found: Error: Can't resolve './controller' in '/Users/heynan0/github/using-catalyst/node_modules/@github/catalyst/lib'
Did you mean 'controller.js'?
BREAKING CHANGE: The request './controller' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./src/index.js 1:0-43

ERROR in ./node_modules/@github/catalyst/lib/index.js 6:0-30
Module not found: Error: Can't resolve './attr' in '/Users/heynan0/github/using-catalyst/node_modules/@github/catalyst/lib'
Did you mean 'attr.js'?
BREAKING CHANGE: The request './attr' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.
 @ ./src/index.js 1:0-43

6 errors have detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

webpack 5.24.0 compiled with 6 errors and 1 warning in 178 ms

I believe I followed all the steps in https://github.github.io/catalyst/guide/you-will-need/ (create a ts project and add experimentalDecorators) yet I get an error when I try to build a new project.

typescript version: 4.7.4
vite version: 3.0.8
catalyst version: 1.6.0

Steps to reproduce

1. Create a new project

npm create vite@latest
Pick vanilla-ts

2. Add experimentalDecorators to tsconfig.json. The full tsconfig look like this:

{
  "compilerOptions": {
    "target": "ESNext",
    "useDefineForClassFields": true,
    "module": "ESNext",
    "lib": ["ESNext", "DOM"],
    "moduleResolution": "Node",
    "strict": true,
    "sourceMap": true,
    "resolveJsonModule": true,
    "isolatedModules": true,
    "esModuleInterop": true,
    "noEmit": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "skipLibCheck": true,
    "experimentalDecorators": true,
  },
  "include": ["src"]
}

3. Create an element in main.ts, this example is from capacitor's doc

import { controller, target } from "@github/catalyst"

@controller
class HelloWorldElement extends HTMLElement {
  @target output: HTMLElement

  greet() {
    this.output.textContent = `Hello, world!`
  }
}

4. Run npm run build. Ts fails to compile:

src/main.ts:4:7 - error TS6196: 'HelloWorldElement' is declared but never used.

4 class HelloWorldElement extends HTMLElement {
        ~~~~~~~~~~~~~~~~~

src/main.ts:5:11 - error TS2564: Property 'output' has no initializer and is not definitely assigned in the constructor.

5   @target output: HTMLElement
            ~~~~~~

Found 2 errors in the same file, starting at: src/main.ts:4

Hi folks,

While I was testing the attributes — trying to work my way around #207 — I found another subtle detail. Manually initialized attributes are never observed.

Catalyst's guide provides the following example for vanilla users: https://github.com/github/catalyst/blob/main/docs/_guide/attrs.md#what-about-without-decorators.

Please notice the second argument there defineObservedAttributes(HelloWorldElement, ['foo']) and compare it with the actual function declaration:

It doesn't expect the second argument (arguments aren't used either).

Now, internally defineObservedAttributes() pulls attribute names from getAttrNames(), which, in turn, pulls them from attrs (WeakMap). The only way to populate attrs is to use @attr decorator:

Maybe I'm missing something, but it seems like there's no way to specify observed attributes without decorators. In any case, it'd be very nice to clarify that 🙂

Thank you!

Hi, this is no doubt a user errror issue rather than a bug, but having read the docs I'm still not sure how to make Catalyst do things.

I'm using:

• The sample from here: https://www.infoq.com/news/2020/12/catalyst-github-web-components/
• Parcel as my build tool (linking my index.ts from index.html)

The markup renders, but I get this error in the console as soon as the page loads: Uncaught TypeError: Failed to construct 'HTMLElement': Please use the 'new' operator, this DOM object constructor cannot be called as a function. at new HelloWorldElement (index.ts:4)

From what I've read this is a common problem when starting developing with web components and people have suggested using the following in your tsconfig.json:

{
	"compilerOptions": {
		"target": "es2015"
	}
}

alternatively

{
	"presets": ["es2015"],
	"plugins": ["transform-custom-element-classes", "transform-es2015-classes"]
}

in .babelrc.

Neither of these things seem to have helped.

Is there an example repo I can look at? Do you have a recommended build pipeline?

Thanks!

Actions cannot be bound in a closed shadow root. This should just work.

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:

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:

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

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

Hey!
First of all, sorry for using GitHub issues for asking questions but not sure what's the best place to ask for help. I want to ask what's the best way to access targets during the initialization process (according to docs,connectedCallback might be too early to query element from the DOM)? Let's say I'd like to setup / start dropzone on a specific target when component is initialized. When / how should I do it?

We need a way to assert that the target exists in the HTML at runtime.

Ran into this with esbuild minified custom element. Names of classes exported from per-element files are not preserved after esbuild minification. While the problem may be overly eager minification, it'd be nice of custom element name could be optionally specified. Something like @controller('my-component').

If a custom event contains a : (for example navigation:keydown), an action attribute like data-action="navigation:keydown:my-controller.handleKeydown" will not work, as it splits on the first : instead of the last one.

https://github.com/github/catalyst/blob/7aaf07f4fd6cea197eb5405b5e611c378946441c/catalyst/src/bind.ts

It is not clear how to use shadowdom, how to get properties and how to use slots. The documentation contains code snippets without the corresponding html.

Right now we drop the Element suffix which makes sense given the precedent of web platform tags which all have constructors ending Element (eg HTMLDivElement). However for other design systems or frameworks it can be common to use different names, eg Controller or Component.

Dropping these would be a breaking change, though, so must be done in 2.0

This project looks great and I'd like to read more. However, the documentation are rendering very poorly on my mobile device. Please improve the documentation template so it renders correctly on smaller screens.

👋 It would be great if it was in the kernel, data prefix is redundant for custom elements.

Using plugins: [ preact() ] in vite.config.js

Tried to create a basic component, as shown in the guide:

import {controller} from '@github/catalyst';

@controller
class TestThingElement extends HTMLElement {

  connectedCallback() {
    this.innerHTML = 'Hello World!';
  }
}

Then use it from HTML:

<test-thing></test-thing>

Nothing gets rendered on the page, and there's no errors. However, if I don't use the @controller decorator, or if I remove the preact plugin, it does work. I looked at the transpiled code and noticed this:

let TestThingElement = controller(_class = class TestThingElement2 extends HTMLElement {

Any idea what's going on here? I assume this is causing problems with how Catalyst uses the class name to determine the custom element tag.

Hi,

There is a function in CATALYST called plotAbundances to plot the population abundances after clustering. I wonder if there is a way to export the raw frequency data of each cell population in each sample? Thanks!

Best,
Jing

It seems that @github/jtml is published on npm but @github/catalyst is published on github packages.
Would be eassier to have both on Github Packages so the npm configation for @github registry is straight forward.

Thanks

Targets should work regardless of open or closed shadows.

Looks like primer searches for fonts in the root directory but we serve the docs site from a sub directory.

Every time use the @controller decorator, i think about whether this is the right name. If omit the details eventually it registers the class in window.customElements and it doesn't matter how you name the class using the endings Element|Controller|Component. Therefore, it seems to me that @customElement is a more appropriate name. Concept of controllers is used in stimulus , but they do not extend the elements. In this regard, Lit is closer to catalyst.

Just read the guide on the main website and found it to be very refreshing and definitely useful in reducing the overhead in working with web components.

But I can't help noticing that most front-end developers will be relying heavily on rendering a list of items on the page and will require a simple and clean manner to go about this.

I see two routes happening already:

• rendering JS in template strings ``
• or using either @github/jhtml as prescribed
• or something like reactjs etc

What is the advise pattern for this obvious outcome of use?

Additional thought: perhaps this is why SSR is mentioned in the guide too?

Thanks.

What are some practices around testing catalyst components?

Maybe catalyst should include some test helpers or a test framework?

I only found it in fact because someone sent me a link to it =]

It would be useful to have this linked in the sidebar:

https://github.github.io/catalyst/guide/you-will-need/

bind only looks at the child elements of a controller for data-action attributes, which means it's impossible to add an action binding to the controller element itself to capture events there.

I think it should be possible to attach events to the controller element itself. I don't believe this would interfere with the "Custom Events" bit mentioned in the docs, as you could still bind a child controller -> parent controllers actions by controller name (assuming the names were different). If the names were the same for some reason, you were rendering recursively that wouldn't work, but I don't believe that works today, as the closest check would prevent the outer controller binding to the inner one:

Not 100% sure if this would be useful to other folks, or if a better pattern is available –but it would be super useful to have a callback for when all targets are connected.

That way one could begin to act on their targets without having to worry about whether their state is ready.

It appears that data-actions defined on an element will be bound by this call:

https://github.com/github/catalyst/blob/main/src/bind.ts#L104-L108

but never cleaned up with a call to removeEventListener. This could lead to memory leaks in situations where catalyst elements are added to and removed from the DOM.

To solve this we probably want execute some code in a wrapped disconnectedCallback that calls removeEventListener for all of the actions that have been bound.

This is maybe related to #117, but not 100% sure on that.

With a component defined like this:

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

@controller
class TestComponentElement extends HTMLElement {

  @attr get booleanProp() {
    return false
  }
  set booleanProp( _v: boolean ) {
    console.dir( `booleanProp type: ${ typeof _v }` )
    console.dir( `booleanProp value: '${ _v }'` )
  }

  connectedCallback() {
    this.booleanProp = true
  }

}

and html like this (note the lack of an initial boolean-prop value):

<test-component></test-component>

I get these log messages:

booleanProp type: boolean
booleanProp value: 'true'
booleanProp type: string
booleanProp value: ''

Changing the html to include an initial value like this:

<test-component boolean-prop='anything'></test-component>

get's rid of that final empty string call:

booleanProp type: boolean
booleanProp value: 'true'

Maybe this is a limitation? Hopefully this isn't another ghost bug, sorry about the last one 🫣

First off, I'm really enjoying this library, so thanks for sharing it! It's been a great transition into custom elements from Stimulus development.

In some recent work, I wanted to extend a native element, HTMLAnchorElement in this case, when building my catalyst-controlled custom element. I ran into a roadblock when using the @controller decorator because it doesn't have a way to specify the extends option.

This could be done manually by the author:

@controller({ extends: 'a' })
class CustomLinkElement extends HTMLAnchorElement {
}

Or inferred by the library code when defining the element:

const ElementMap = {
  'HTMLAnchorElement': 'a',
 ...etc
};

// register.ts
let extends = ElementMap[Object.getPrototypeOf(classObject).name];
window.customElements.define(name, classObject, { extends });

I like the ability extend native elements in order to limit the extraneous markup to add functionality, but I understand if this feels out of scope for a stimulus-type replacement.

Repro’d in Safari 14.0.3 and Chrome 89.0.4389.82

e.g. https://github.github.io/catalyst/reference/#attr just says:

Reference guide

Reference guide

Find the reference guide for all our exported functions below

Given

@targets filters: HTMLElement[]
@target allFilter: HTMLElement

Currently this is the markup

<div data-target="foo-controller.filters foo-controller.allFilter"></div>

I propose that we add data-targets, like so:

<div data-targets="foo-controller.filters" data-target="foo-controller.allFilter"></div>

In this case we see from the markup that this is supposed to be the only one allFilter, but there should be multiple filters.

I think data-targets will

1. Match with the decorator thus make it clearer that to find a collection you need @targets in TS.
2. Provide better context when looking at the markup. When refactoring unfamiliar, JS we often have to check "is this thing rendered more than one time?"

Issue: The Reference page was removed here 8ea7208
as a solution to #126, but unfortunately there are still links to this broken page within the main site header and banner image.

This is because we only run the @bind decorator once when the JS loads and don't make sure to re-bind new actions in HTML that is dynamically injected into the page.

Hi

I'm working on a Catalyst template for webcomponents.dev

I built this one
https://webcomponents.dev/edit/YclS8tjAV7ea8I87DIMG

For future reference this is the code:

import { controller, target } from "@github/catalyst";

const template = document.createElement("template");
template.innerHTML = `<style>
    * {
      font-size: 200%;
    }

    span {
      width: 4rem;
      display: inline-block;
      text-align: center;
    }

    button {
      width: 4rem;
      height: 4rem;
      border: none;
      border-radius: 10px;
      background-color: seagreen;
      color: white;
    }
  </style>
  <button data-action="click:my-counter#dec">-</button>
  <span data-target="my-counter.value"></span>
  <button data-action="click:my-counter#inc">+</button>`;

@controller
class MyCounterElement extends HTMLElement {
  @target value: HTMLElement;

  count: number = 0;

  connectedCallback() {
    this.attachShadow({ mode: "open" });
    this.shadowRoot.appendChild(template.content.cloneNode(true));
    this.update();
  }

  inc() {
    this.count++;
    this.update();
  }

  dec() {
    this.count--;
    this.update();
  }

  update() {
    this.value.textContent = "" + this.count;
  }
}

Visuals are ok.
but clicks don't work.

If I remove the shadowroot then clicks work.

Note: I tried to use @github/jtml referenced in the documentation but it's not open to public yet (?)

Thanks
Georges

cc @koddsson

Hey there 👋🏻

I created a couple of custom elements using catalyst and it worked just fine, once a managed to get decorators to work with Rails 6 + webpacker. However, once I deployed it to heroku, I got the following error in the console:

register.js:15 Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': "n" is not a valid custom element name
    at https://my-app.herokuapp.com/packs/js/application-55c5e8e27a6ea838e36d.js:2:217687
    at O (https://my-app.herokuapp.com/packs/js/application-55c5e8e27a6ea838e36d.js:2:217700)
    at Module.<anonymous> (https://my-app.herokuapp.com/packs/js/application-55c5e8e27a6ea838e36d.js:2:241463)
    at n (https://my-app.herokuapp.com/packs/js/application-55c5e8e27a6ea838e36d.js:2:110)
    at https://my-app.herokuapp.com/packs/js/application-55c5e8e27a6ea838e36d.js:2:911
    at https://my-app.herokuapp.com/packs/js/application-55c5e8e27a6ea838e36d.js:2:921
(anonymous) @ register.js:15
O @ controller.js:25
(anonymous) @ nearby_facilities_inputs.ts:4
n @ bootstrap:19
(anonymous) @ bootstrap:83
(anonymous) @ application-55c5e8e27a6ea838e36d.js:2

That error broke the entire application.js. My nearby_facility_inputs.ts is this:

import { controller, targets, target, attr } from "@github/catalyst";
import { render, html } from "@github/jtml";

@controller
export class NearbyFacilitiesElement extends HTMLElement {
  @targets inputs: NearbyFacilityElement;
  @target addButton: HTMLButtonElement;

  addFacility(e) {
    e.preventDefault();

    const el = document.createElement("nearby-facility");

    const index = new Date().getTime();

    ["name", "distance", "icon"].forEach((field) => {
      el.setAttribute(
        `data-input-name-for-${field}`,
        this.getAttribute(`data-input-name-for-${field}`).replace(
          /{index}/,
          index
        )
      );
      el.setAttribute(
        `data-placeholder-for-${field}`,
        this.getAttribute(`data-placeholder-for-${field}`).replace(
          /{index}/,
          index
        )
      );
    });

    el.setAttribute(
      "data-remove-label",
      this.getAttribute("data-remove-label")
    );
    this.appendChild(el);
  }
}

@controller
export class NearbyFacilityElement extends HTMLElement {
  @attr name = "";
  @attr distance = "";
  @attr icon = "";
  @target iconElement: HTMLElement;

  connectedCallback() {
    this.render();
  }

  placeholderFor(field: string) {
    return this.getAttribute(`data-placeholder-for-${field}`) || field;
  }

  inputNameFor(field) {
    return this.getAttribute(`data-input-name-for-${field}`) || field;
  }

  render() {
    const template = html`
      <div class="input-group mt-3">
        <input
          name="${this.inputNameFor("name")}"
          placeholder="${this.placeholderFor("name")}"
          class="form-control"
          value="${this.getAttribute("data-name")}"
        />
        <input
          name="${this.inputNameFor("distance")}"
          placeholder="${this.placeholderFor("distance")}"
          class="form-control"
          value="${this.getAttribute("data-distance")}"
        />
        <input
          name="${this.inputNameFor("icon")}"
          placeholder="${this.placeholderFor("icon")}"
          class="form-control"
          onkeyup="${this.updateIcon.bind(this)}"
          value="${this.getAttribute("data-icon")}"
        />
        <div class="input-group-text" style="width: 40px;">
          <i
            data-target="nearby-facility.iconElement"
            class="bi bi-${this.getAttribute("data-icon")}"
          ></i>
        </div>
        <button onclick="${this.remove.bind(this)}" class="btn btn-danger">
          ${this.removeLabel()}
        </button>
      </div>
    `;
    render(template, this);
  }

  updateIcon(e) {
    const value = e.target.value;
    this.iconElement.setAttribute("class", `bi bi-${value}`);
  }

  removeLabel() {
    return this.getAttribute("data-remove-label") || "Remove";
  }

  remove() {
    this.parentElement.removeChild(this);
  }
}

I am not sure this is a catalyst specific error. I suppose not. I think this is a transpilation error, but I am unfamiliar with the whole webpack babel thing I decided to ask for some directions here.

Also, I am wondering how could I make those custom elements not break the entire application.js - perhaps I could wrap some code in a try/catch?

Thank you in advance ❤️

I think there's a regression in the typedefs for version 1.3.1.

https://github.com/github/catalyst/pull/190/files#diff-0609e410282ce718628c6b5977007b6a14a3edef8f178c61d749d134249ae52f

Changing the typedefs sorts it for me:

/**
 * Target is a decorator which - when assigned to a property field on the
 * class - will override that class field, turning it into a Getter which
 * returns a call to `findTarget(this, key)` where `key` is the name of the
 * property field. In other words, `@target foo` becomes a getter for
 * `findTarget(this, 'foo')`.
 */
- export declare function target<K extends string>(proto: Record<K, unknown>, key: K): Record<K, unknown>;
+ export declare function target<K extends string>(proto: Record<K, unknown>, key: K): void; 
/**
 * Targets is a decorator which - when assigned to a property field on the
 * class - will override that class field, turning it into a Getter which
 * returns a call to `findTargets(this, key)` where `key` is the name of the
 * property field. In other words, `@targets foo` becomes a getter for
 * `findTargets(this, 'foo')`.
 */
- export declare function targets<K extends string>(proto: Record<K, unknown>, key: K): Record<K, unknown>;
+ export declare function targets<K extends string>(proto: Record<K, unknown>, key: K): void;
//# sourceMappingURL=target.d.ts.map

node_modules/@github/catalyst/lib/target.d.ts

Looks like the lib folder wasn't published in 1.4.0.

Noticed this in a local install with npm 8.5.0 but here's a screen cap from jsdelivr

https://www.jsdelivr.com/package/npm/@github/catalyst?version=1.4.0

vs
https://www.jsdelivr.com/package/npm/@github/catalyst?version=1.3.2

The attributeChangedCallback function is called whenever an observed attribute is set, added or removed. The attributeChangedCallback is called with the raw values of the attribute name, old value and new value.

This can be surprising when using the @attr decorator which gives conveniences over the attributes by allowing for camel-cased names eliding their data- prefix, and allowing types other than string. It can also be surprising to see attributeChangedCallback being called with null when you type an @attr as string.

For example:

@controller 
class FooBarElement extends HTMLElement {

  @attr foo = true

  attributeChangedCallback(name, oldValue, newValue) {
    console.log(name, oldValue, newValue)
  }

}

When this element first connects, attributeChangedCallback('data-foo', null, '') will fire. attributeChangedCallback will never fire with 'foo' as the name, because that is a Catalyst concept and not a WC concept. The same is true of the boolean type; attributeChangedCallback will never fire with booleans for values. This means developers have to suddenly work around all of the conveniences @attr offers them:

@controller 
class FooBarElement extends HTMLElement {

  @attr foo = true

  attributeChangedCallback(name, oldValue, newValue) {
    if (name === 'data-foo') { // Smell: `data-foo` is the name of the attribute but we call if `foo` everywhere else in the file
      if (newValue === null) { // Smell: this should really be `if (oldValue === false)` to align with our expected types
        doStuff();
      }
    }
  }

}

Possible Solutions

1. Document this gotcha and just let developers deal with it.

• This burdens developers with the extra concepts they have to keep in their brain
• It demonstrates that this is a leaky abstraction
• The code they eventually have to write still smells

2. Add extra code to call attributeChangedCallback with the mapped name, oldValue, newValue

• Potentially harmful: attributeChangedCallback has a fixed signature where it is only called with string|null and we're abusing that contract.
• This will cause the attributeChangedCallback to fire far more often, effectively double for each change to an attr mapped value. Use cases which do not care about the argument values will see more - effectively redundant - calls.

3. Add extra code to call a new function (maybe attrChangedCallback) with the mapped name, oldValue, newValue.

• This means further divergence from web component spec.
• More to document, more stuff that Catalyst is responsible for.

4. Add a function which can be given the 3 arguments and map it back to prop names (e.g. const [realName, realOldValue, realNewValue] = this.attributeToAttr(name, oldValue, newValue))
   • I hate it
   • Still burdens developers with extra concepts
   • Still demonstrates the leaky abstraction
   • Still requires more documentation, more stuff that Catalyst is responsible for.
5. Add events which are automatically dispatched when attributes changed, which can be listened for. changing HelloWorldElement's @attr name = '' value could emit hello-world-name-changed.
   • It's still idiomatic to web components... sort of
   • Elements become very noisy
   • Required elements binding to their own events, so they're not in control of their own state.
   • Still requires more documentation

The attrs doc page, for example, says one can

import {initializeAttrs, defineObservedAttributes} from '@github/catalyst

However a look at index.ts shows this is not exported. Trying to run these examples fails.