Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for composed microsoft/fast-foundation templates and styles #184

Open
sonntag-philipp opened this issue Jul 26, 2022 · 4 comments
Open

Comments

@sonntag-philipp
Copy link

Description

The custom elements analyzer is a great tool to analyze component libraries. Currently, I'm using it to analyze my component library and automatically generate a API documentation that can be browsed in Storybook. The components in the library are based on microsoft/fast-foundation.

Here is an example how I use the FAST Foundation Button implemented by microsoft/fast-foundation to compose my own button.

import { css } from "@microsoft/fast-element";
import { Button, buttonTemplate } from "@microsoft/fast-foundation";

export const buttonStyles = css`
    button {
        padding: 1rem 1.5rem 1rem 1.5rem;
        border: none;
        cursor: pointer;
    }

    button:hover {
        background-color: rgba(0, 0, 0, 0.1);
    }

    button:active {
        background-color: rgba(0, 0, 0, 0.2);
    }
`;

/**
 * @tag odui-button
 */
export class OduiButton extends Button {}

export const oduiButton = Button.compose({
    baseName: "button",
    baseClass: OduiButton,
    styles: buttonStyles,
    template: buttonTemplate,
    shadowOptions: {
        delegatesFocus: true,
    },
});

Problem

One thing that can be noticed here is that the templates and styles are not directly contained in the component class. This creates a contrast to base libraries where it's only possible to define the styles directly in the component (e.g. Google Lit).

This almost works fine with the current analyzer as it only extracts the members, attributes and events out of base classes. All of those properties are implemented and provided by the class implementation. This is also the case for composed FAST components when they are declared in a class like the example above.

The problems begin with properties that are defined in the templates. cssParts and slots that are defined in the composed template are currently not contained in the generated manifest. This generates a huge gap in the component's documentation.

Solution proposal

My proposal for a solution to this problem would be to add JSDoc annotations to the html and css string literals. This would look something like this:

/**
 * Template of the button component.
 * @slot start - Content which can be provided before the button content
 * @slot default - Content which is displayed in the button
 * @slot end - Content which can be provided after the button content
 */
export const buttonTemplate = html`<button>
    <slot name="start"></slot>
    <slot></slot>
    <slot name="end"></slot>
</button>`;

This change would require the analyzer to look for .compose() calls and to include available JSDocs of the used arguments.

Lazy styles and templates
FAST also has a feature that enables the developer to lazily define templates and styles. I'm not really sure if this feature can be supported by a static code analysis that has no runtime knowledge like the cem analyzer, so I'm open for suggestions.
I think we should implement the same logic for those dynamic parts that just analyzes the JSDoc annotations and ignores the content of the template literal.

Future proof
The only part that is announced to change is the compose() call. It is going to be replaced by FASTElementDefinitions or something similar. For a deep look into the changes, take a look into this thread that discusses the future changes of FAST.

I'm really looking forward for your feedback on this topic before I start contributing to a solution for this problem. Thanks!

@thepassle
Copy link
Member

The analyzer doesn't analyze css or html tagged template literals for lit, either. Would it be an option to annotate the slots and cssparts on the class? https://custom-elements-manifest.open-wc.org/analyzer/getting-started/#supported-jsdoc

As for analyzing the compose function, I suppose you could try to analyze that, but since it's highly dynamic you're probably going to run into lots of edge cases. If you want to try to implement this, you should have all the information available in the plugin API.

@sonntag-philipp
Copy link
Author

Thank you for the fast answer @thepassle!

Would it be an option to annotate the slots and cssparts on the class?

Something like this was also on my mind at first. But then I thought that the documentation of the template - which the csspart and slot annotations are in my opinion - should be defined right next to the declaration of the template.
Otherwise it would be very possible to have not just no custom element manifest data available, but much more wrong data as the component may be composed with a different template in further steps.

The documentation right next to the literals would also be applicable for Google Lit or other frameworks that use template literals. I don't know if this is interesting or an use case for other developers but it could be if Lit decides to add similar technologies like FAST.

As for analyzing the compose function, I suppose you could try to analyze that, but since it's highly dynamic you're probably going to run into lots of edge cases.

Yes, that's true. Good point. I think the solution that I could create would be really error prone and not a reliable solution due to the dynamic capabilities of FAST. This is why I contacted the FAST team yesterday. It seems like they are planning to create an integration that automatically creates the manifest into the FAST Foundation and the FAST CLI.

For now, a simple solution for my initial problem of missing out the documentation of base components would be to create a plugin for the analyzer that also extracts css parts and slots from the base. The analyzer does not do this by default, but I think I can work with the post processing script that already extracts members etc. and extend it easily :)

@yinonov
Copy link

yinonov commented Feb 26, 2023

would be really error prone and not a reliable solution due to the dynamic capabilities of FAST

@sonntag-philipp I imagine this also includes the custom prefixing featured by the FAST design system provider?

@sonntag-philipp
Copy link
Author

Yes. I would've created a static analysis of a highly dynamic compose method. This would include many edge cases that had to be defined and thought through prior to the implementation or otherwise we potentially would've got many errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants