feat: Add decorator for configuring CSS `CustomStateSet`s

[KingOfTac]

Pull Request

📖 Description

This PR adds a new decorator for configuring boolean properties as CSS custom states. The decorator will attempt to call attachInternals on the element if it hasn't already and then create the configured property as an observable property with a custom setter that handles setting and removing the custom state from the element's ElementInternals.states set.

update: The bulk of the decorator's logic has been moved to a separate CSSStateDefintion class in order to more easily handle notifying subscribers of changes and to support propertyChanged callbacks for the decorated properties. The implementation of the class follows a similar pattern used by @attr.

Usage

@customElement({
  name: 'my-el',
  styles: css`
    :host(:state(foostate)) { ... }
  `
})
class MyEl extends FASTElement {
  @cssState
  private fooState: boolean = false;
}

🎫 Issues

👩‍💻 Reviewer Notes

Some open questions:

• I'm not sure if attaching internals within CSSStateDefinition.setValue is the best place for it. The issue is that in order to know if ElementInternals has been attached to the element, it needs to be assigned to a property and the logic to detect it cannot live in the decorator due to the decorator not having access to the component instance. We could implement a similar solution as @attr where it implements a metadataLocator to set the configuration for the component's type and then later instantiates the custom states during element construction. This is a larger change that would affect FASTElement.

• In order to more consistently detect if ElementInternals has already been attached to the source element, I think either FASTElement or ElementController would need to be updated with an optional elementInternals property. An override for attachInternals would likely be needed to set this new property so that component authors can just call this.attachInternals() and not need to worry about manually setting a class member for it.

• I'm not sure the components folder is the best location for this, perhaps it is more related to styling?

• I haven't seen CustomStateSets used much in the wild yet and any usage I have seen has been limited to boolean properties since custom states are similar to HTML boolean attributes where their true/false states correlate with the existence of the attribute or in this case, the value passed to state() selector causing styles to be applied. If support for other primitive types should exist, then we should discuss what that looks like.

• It would seem TypeScript (or the version FAST uses) does not yet support CustomStateSet on ElementInternals so this PR also includes a small type polyfill for ElementInternals.states. fast-element does not have a global.d.ts file which is typically where global interfaces reside, so for now I defined the modified interface within the same file as the decorator.

📑 Test Plan

I'm having difficulty adding tests around this change due to some issues with the test environment. Perhaps @radium-v could help take a look?

✅ Checklist

General

• [ ] I have included a change request file using $ yarn change
• [ ] I have added tests for my changes.
• [x] I have tested my changes.
• [ ] I have updated the project documentation to reflect my changes.
• [ ] I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

A nice enhancement would be to add an optional configuration to the decorator similar to @attr's configuration where the name of the applied state in CSS can be defined differently than the property on the class.

[nicholasrice]

• In order to more consistently detect if ElementInternals has already been attached to the source element, I think either FASTElement or ElementController would need to be updated with an optional elementInternals property. An override for attachInternals would likely be needed to set this new property so that component authors can just call this.attachInternals() and not need to worry about manually setting a class member for it.

One option here would be to expose state on the ElementDefinition similarly to attributes. When the definition is composed, it could make available the states to the elementController, which could attachInternals and expose the internals from the controller. But I think you're right, you would want to guard against a multiple attachment scenario, just returning the already attached internals object when invoked multiple times.

Another thing I notice is that this decorator mixes observability with css state. One thing to consider here is that decorators can be used together, so authors should be able to do:

@observable
@cssState
property: string = ""

// or
@attr
@cssState
attr: string = ""

This approach would keep the behaviors more isolated and allow usage of CSS state w/o the overhead of property observability if desired. You might need to upgrade TS to do that though... I'm not sure if this version exposes the underlying property descriptor or not. You would probably need to update observable to not just clobber existing property descriptors as well.

[KingOfTac]

I'm not sure if this version exposes the underlying property descriptor or not

Looking at the docs, it looks like property decorators don't get the descriptor passed to them so chaining may be difficult but I don't think it's impossible.

In my initial testing for composing the different decorators, which ever decorator is applied to the property second clobbers the first. I think both this decorator and observable would need to first check if an accessor already exists and reflect calls to it rather than just overriding it completely, but I'm not entirely sure if it can be done with the Reflect API.

[KingOfTac]

Some new things that have come up around this.

I think @nicholasrice's suggestions on making custom states part of the definition and ElementInternals part of the controller is the right approach, especially since we'll likely want to control access to ElementInternals and guard against double attachment.

What I'm not sure about is making both this new decorator and observable composable. I managed to get a working prototype of composable decorators that don't just clobber the Accessor, however it could potentially add more overhead than just including observability in the new decorator and making it a configurable option. This is because for each decorator in the chain to not clobber the Accessor created by the previous one, each decorator would need to hold a reference to the existing PropertyDescriptor, override it with it's own implementation and reflect get/set calls up to the previous version.

The benefit to making observable and other decorators composable is that it makes them more friendly to user defined decorators. The tradeoffs are the previously mentioned overhead of holding descriptor references and that we can't fully test for and anticipate every possible scenario in which FAST's decorators could be composed with other decorators so it could add user frustration if the behavior is not documented well.

The second tradeoff above could be alleviated by exporting some utilities for creating composable decorators which would also enable FAST to have a lot of control over user decorators created with said utilities, but I'm not sure what that would look like yet.

@janechu I'd love to hear your thoughts on all of this. I'm not sure what the timeline is for getting v2 out the door, so perhaps we keep this change in a draft state and iterate on it for v3.

[janechu]

I believe @radium-v is going to have more feedback on the actual implementation, for 2.0.0 launch we're still in progress on updating the documentation site, hopefully that will be done very soon. If this is going to go in, either as part of 2.0.0 or as a feature release I'm not particular, however it should have a section on the site. I'll link to the updated documentation PR once it's up.

[radium-v]

Hi 👋🏼

I'm still learning about CustomStateSet as today is the first time I've actually looked at it, now that states are supported everywhere. I'm not sure if I can help make any decisions here yet, but I can at least provide some info on what we've been doing with ElementInternals in Fluent web components.

---

anecdote starts here I actually already found a use case for ElementInternals.state - here's a small example, but it's pretty useful: https://github.com/radium-v/fluentui/blob/58c0852c0a99f02d7ec64b3eface03340dfee103/packages/web-components/src/field/field.ts#L92C1-L115C1

In this particular case, support for :host(:has()) is out of scope for the spec and currently only works in Safari. I need some way to style the component when it has :focus-within, but only if it's from a slotted element with :focus-visible. By applying --focus-visible as a state, I can use this selector:

:host(:is(:--focus-visible, :state(--focus-visible)):focus-within) {

...Which is admittedly pretty insane, but it does work. It also lets me avoid using an arbitrary attribute.

There are of course compatibility concerns, which is why the state is --focus-visible instead of focus-visible, since some older browsers only support :-- as the pseudo-selector, which is deprecated in favor of :state().

anecdote stops here

---

For Fluent components, I've been sticking to elementInternals as the property name. Certain other methods and properties are reflected, including setFormValue(), setValidity(), form (getter), and labels (getter), but these reflections aren't completely set in stone, and are specific to the component at hand (read: mostly arbitrary).

Additionally, if ElementInternals is going to be added to the controller, this may likely be a major breaking change for existing components which define and use it already. So far, that's <fluent-button>, <fluent-compound-button>, <fluent-menu-button>, <fluent-toggle-button>, and <fluent-spinner> (with at least <fluent-checkbox>, <fluent-field>, and <fluent-text-input> currently in PR, and more to come).

• Button: https://github.com/microsoft/fluentui/blob/master/packages/web-components/src/button/button.ts
• Spinner: https://github.com/microsoft/fluentui/blob/master/packages/web-components/src/spinner/spinner.ts
• TextInput: https://github.com/radium-v/fluentui/blob/users/radium-v/web-components-v3-text-input/packages/web-components/src/text-input/text-input.ts

[KingOfTac]

this may likely be a major breaking change for existing components which define and use it already

For this reason, I think this change should be part of v3. v2 is too close to release for us to be introducing breaking changes especially considering v2 beta has been stable for quite some time now.

Now, we could add elementInternals to FASTElement rather than the controller which should soften the impact this change has a bit, but I'm not sure what the implications of that would be yet.

On the compatibility aspect I think we should be able to detect which version of the spec is supported within the decorator and set the state as a string or dashed-ident accordingly. Writing the actual styles would be up to the component author since the decorator is only concerned with adding/removing the state.