janechu commented 2 months ago

Library

Web Components (@fluentui/web-components)

Describe the feature that you would like added

Registering CSS properties

Problem

Currently there is no way to know which CSS properties are actually being used in imported web components.

Current solution

Implementations utilizing the web components currently define all CSS properties assuming that any of the available CSS properties might be used. The issue with this is there are already a large number of CSS properties that exist in the library, and that number will continue to increase. This is causing some performance issues on initial page render.

Proposed solution

Wherever the user imports a definition for a web component they should optionally be able to register the components CSS properties:

note: This should make the addition of registering CSS properties a non-breaking change and users can opt out if they prefer to define CSS properties themselves.

import "@fluentui/web-components/button";
import "@fluentui/web-components/button/register-css-properties";

If the user intends to get the CSS properties that have been added, they will have two methods available, get and notify.

import cssPropertyRegistry from "@fluentui/web-components/css-property-registry";

// Gets a map of registered CSS properties
cssPropertyRegistry.get();

// Notify when new css properties get registered for dynamic imports
cssPropertyRegistry.notify((newCssproperty) => {
  // do something with the newCssProperty
});

Proposed implementation

Creation of a build script that generates a *.css-properties.ts file per component that adds each CSS property imported in *.styles.ts from design-tokens.js.
Add an export of cssPropertyRegistry which will allow adding, getting all registered CSS properties, and a notify which accepts a callback which any newly added CSS properties will trigger.

Potential issues

If there is a duplication of the dependency, this will cause errors in browser as it is assumed that there is only one registry, any attempt at using the registerProperty when a CSS property has already been registered will result in a console error.
Similar to the previous point, if any other part of the code attempts to use the registerProperty API with the same CSS property name it will result in a console error.
The registerProperty API is new on the nightly builds of firefox, and not yet on the current version. It is available on all other major browsers, see the browser compatibility table for details.

Have you discussed this feature with our team

chrisdholt

Additional context

No response

Validations

[X] Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Priority

Normal

codysorgenfrey commented 2 months ago

Do you envision this supporting multiple targets for CSS variable definitions? For instance; will this system work for the following structure?

<body> /* Root level theme */
  <div id="some-stuff">
    <div id="second-theme-target"> /* A different theme applied here */
      ...
    </div>
  </div>
</body>

I'd expect to be able to register/notify/add different variables for each system independently.

janechu commented 2 months ago

Do you envision this supporting multiple targets for CSS variable definitions? For instance; will this system work for the following structure?
<body> /* Root level theme */
  <div id="some-stuff">
    <div id="second-theme-target"> /* A different theme applied here */
      ...
    </div>
  </div>
</body>
I'd expect to be able to register/notify/add different variables for each system independently.

The CSS variables get applied at the global level (the window), so it will cascade as normal, we will be using the inherits: true option when using registerProperty, you could define on any element and it will override.

Edit: I should say this would only apply global defaults, it will be up to the developer to specify any overrides, and the registration will not be configurable. We are trying to avoid any additional complications about knowing DOM structure etc.

spmonahan commented 2 months ago

Re: Potential Issues

Is the lack of wide spread registerProperty support really a problem? In my experience with this API the main benefit is being able to define type information for a custom property but since this is not something we can do today (and given browser support isn't something we will be able to rely on in the near term) it doesn't seem like a hard requirement for this feature.

In the absence of registerProperty defining a custom property on :root should be equivalent behavior to what we have today?

I'd like to hear your thoughts on interop with Fluent React. There has been a good amount of work in the past few months to improve the React <--> Web Components interop story for Fluent React and a feature like this feels to me like something that should be usable in both Fluent WC and Fluent React.

I don't know that Tokens is the correct home for it but this is an example of a package that is shared between the two implementations today. Should this feature live in a place that is easily consumable by both Fluent React and Fluent WC and define an implementation agnostic API?

janechu commented 2 months ago

Re: Potential Issues

Is the lack of wide spread registerProperty support really a problem? In my experience with this API the main benefit is being able to define type information for a custom property but since this is not something we can do today (and given browser support isn't something we will be able to rely on in the near term) it doesn't seem like a hard requirement for this feature.

In the absence of registerProperty defining a custom property on :root should be equivalent behavior to what we have today?

I'd like to hear your thoughts on interop with Fluent React. There has been a good amount of work in the past few months to improve the React <--> Web Components interop story for Fluent React and a feature like this feels to me like something that should be usable in both Fluent WC and Fluent React.

I don't know that Tokens is the correct home for it but this is an example of a package that is shared between the two implementations today. Should this feature live in a place that is easily consumable by both Fluent React and Fluent WC and define an implementation agnostic API?

In regards to registerProperty, I don't think it's a major issue especially since the web components v3 is currently in pre-release, but I felt it worth at least mentioning to make sure we have a complete understanding of the current web browser adoption. It's not a reason to not do this feature.

Yes we could define the custom property on :root if it is an issue.

I've spoken with a few people on the Fluent React side of things, our current conclusions are:

Flight this in web components v3 beta.
Do more performance testing (we've done some preliminary tests, loading many buttons on a page for example, but no 'real world app' scenarios with deep nesting for example).
Re-convene prior to launch of web components v3 with learnings and see what we can extract from the web components package into another package, building on the work that is currently being done with the Design Token updates which are also being worked on so that React can also use this utility. Perhaps the tokens package is the right place!
If we see the need, potentially consider a build time solution as well, both may be needed depending on the app and the use cases.

mohamedmansour commented 2 months ago

One of my main objectives is to use Web Platform natively, having any token system in JS is going a step backwards. So having a build-step solution where it extracts the tokens used in the FAST Web Components web app, so we can place them in the index.html in our bundler would be great. That would eliminate all the token system at startup.

There is one problem with that though, is that we need to additionally have custom themes using createLightTheme and createDarkTheme on runtime so that could probably be loaded on demand dynamically.

janechu commented 2 months ago

One of my main objectives is to use Web Platform natively, having any token system in JS is going a step backwards. So having a build-step solution where it extracts the tokens used in the FAST Web Components web app, so we can place them in the index.html in our bundler would be great. That would eliminate all the token system at startup.

There is one problem with that though, is that we need to additionally have custom themes using createLightTheme and createDarkTheme on runtime so that could probably be loaded on demand dynamically.

Using the Web Platform natively is our objective as well, having said that at run time there is no current way to get all CSS properties that are being used. The first problem we are solving is allowing a user to know that during run-time, this doesn't seem like a problem you need solved, but you might be interested in a build time solution which we'll also be prototyping since there is so much interest.

KingOfTac commented 1 month ago

I ran into some similar issues on another project and came up with a FAST stylesheet behavior to handle some of the problems I was facing. This doesn't quite solve the issue of notifying users of the registered properties but it does enable moving the registration of the properties to the component's styles that "own" them.

export class RegisteredPropertyBehavior implements HostBehavior {
  private propertyDefintionCache = new Map<string, PropertyDefinition>();

  public readonly properties: PropertyDefinition[];

  constructor(...properties: PropertyDefinition[]) {
    this.properties = properties;
  }

  connectedCallback(): void {
    if (!CSS.registerProperty) {
      console.warn('CSS.registerProperty is not supported in this environment');

      return;
    }

    const { properties } = this;

    for (const property of properties) {
      if (this.propertyDefintionCache.has(property.name)) {
        console.warn(`Property ${property.name} has already been registered`);
        return;
      }

      this.propertyDefintionCache.set(property.name, property);

      try {
        CSS.registerProperty(property);
      } catch (error) {
        console.warn(error);
      }
    }
  }
}

microsoft / fluentui

[Feature]: Registering CSS properties #31182

Library

Describe the feature that you would like added

Registering CSS properties

Problem

Current solution

Proposed solution

Proposed implementation

Potential issues

Have you discussed this feature with our team

Additional context

Validations

Priority