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

Idea: allow configuring framework plugins (how to adjust core plugin output) #189

Open
jrencz opened this issue Nov 1, 2022 · 0 comments

Comments

@jrencz
Copy link

jrencz commented Nov 1, 2022

For now plugins are "all or nothing". In my case it Lit:

{
    litelement: true,
}

Here's a case why it was not good enough for me, how I solved it and what I propose to make it more approachable:

What was not OK with defaults for me

I made a decision to amend my LitElements with an override of createProperty:

@customElement('my-element')
export class MyElement extends LitElement) {
  static override createProperty(name, options) {
    super.createProperty(name, kebabAttribute(name, options));
  }

  // quite a lot of properties which some are 2 or more words and
  // I didn't fancy the default policy of "just lowercase the property"
  // I also want not to express them all because it makes the bundle
  // bigger and code harder to read
}
export const kebabAttribute = (name, options) => {
  if (
    typeof name !== 'string'
    || !/[A-Z]/.test(name)
    || typeof options?.attribute === 'string'
    || options?.attribute === false
  ) {
    return options;
  }

  return {
    ...options,
    attribute: kebabCase(name),
  };
}

LitPlugin (obviously) does not know about this decision because decision is executed in runtime.

What I did

Step 1: Decompose

+ import { customElementDecoratorPlugin } from '@custom-elements-manifest/analyzer/src/features/framework-plugins/decorators/custom-element-decorator.js'
+ import { methodDenyListPlugin } from '@custom-elements-manifest/analyzer/src/features/framework-plugins/lit/method-denylist.js';
+ import { memberDenyListPlugin } from '@custom-elements-manifest/analyzer/src/features/framework-plugins/lit/member-denylist.js';
+ import { propertyDecoratorPlugin } from '@custom-elements-manifest/analyzer/src/features/framework-plugins/lit/property-decorator.js';
+ import { staticPropertiesPlugin } from '@custom-elements-manifest/analyzer/src/features/framework-plugins/lit/static-properties.js';

export default {
-  litelement: true,
  plugins: [
+    customElementDecoratorPlugin(),
+    methodDenyListPlugin(),
+    memberDenyListPlugin(),
+    propertyDecoratorPlugin(),
+    staticPropertiesPlugin(),
  ]
}

It gave the same result (even though I know it's at a cost of maintenance if internal structure would change), so

Step 2: Try configuring propertyDecoratorPlugin

Aaaaaand... it's gone.
Plugin is written in such way that it takes no config.

Step 2.1: Reimplement propertyDecoratorPlugin

Shortening things up:

if(attributeName) {
attribute.name = attributeName;
field.attribute = attributeName;
} else {
field.attribute = field.name;
}
if(reflects(propertyOptions)) {
field.attribute = attribute.name;
field.reflects = true;
}

        if(attributeName) {
          attribute.name = attributeName;
          field.attribute = attributeName;
        } else {
-         field.attribute = field.name;
+         field.attribute = kebabCase(field.name);
        }


        if(reflects(propertyOptions)) {
-         field.attribute = attribute.name;
+         field.attribute = kebabCase(attribute.name);
          field.reflects = true;
        }

(BTW: I think there's a bug in reflect logic - it assumes that reflected attribute is given explicitly. I need to think about it some more before I can be sure)

So: I got what I wanted, but it's WAAAAY too much bound to the internal implementation.
It's not what I want to maintain or leave my colleagues to maintain, so...

Step 3: "Fix it later"

I wrote my own plugin to adjust what I don't like about the default one:

import kebabCase from 'just-kebab-case';

export default function kebabCaseAttributesPlugin() {
  const name = 'kebabCase-attributes';
  const is = expectedKind => ({kind}) => kind === expectedKind
  const has = expectedProp => member => expectedProp in member;
  const attributeMatches = regexp => memberWithAttribute => regexp.test(memberWithAttribute.attribute)

  return {
    name,
    moduleLinkPhase({moduleDoc, context}) {
      if (moduleDoc.kind !== 'javascript-module') {
        return;
      }

      moduleDoc.declarations
        .filter(is('class'))
        .forEach(({members}) => members
          .filter(is('field'))
          .filter(has('attribute'))
          .filter(attributeMatches(/[A-Z]/))
          .forEach(member => {
            const kebabCased = kebabCase(member.attribute);
            if (context.dev) {
              console.log(`[${name}] ${member.attribute} -> ${kebabCased}`);
            }
            member.attribute = kebabCased;
          })
        );
    }
  }
}

I'm leaving it in here for reference, maybe someone will benefit from it.

I'm a rookie in WebComponents and I started using the analyser 2h ago, but I'm a JS dev with 10y+ exp, so it was quite easy for me (btw - kudos for plugin architecture, It was that easy because I made use of terrific idea to allow moduleLinkPhase), but it's not exactly the greatest DX for people with less experience.

What I propose to discuss

  • define LitPluginOptions
  • allow passing it instead of true to litelement
  • distribute values from LitPluginOptions to plugins

Or maybe include some examples of how to adjust the output for one's needs as I did.
Either way discussing what might get configured is a first step for both.

Truth to be said: providing tips on how to adjust it with ones own plugin promotes building more plugins and building awareness that it's not that hard to build plugins on one's own, so maybe the latter is even better than allowing configuration. For sure it's less of a maintainer load.

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

1 participant