Skip to content

Clean code guidelines #434

@fortesp

Description

@fortesp

What problem does this feature solve?

Problem
Components do not follow a consistent structure. Refactoring is needed. Public API and internals are mixed. Naming, ordering, and visibility vary by file. This violates clean code standards which difficult code reading, making it harder to develop on.
i.e. six-select vs six-button, etc etc.

Use case, context, rationale
Teams maintain many components with frequent contributors. New and existing developers waste time finding props, events, and methods. A repository-wide, enforced layout and naming convention removes ambiguity. It also aligns with the project goal of a small, clear API surface by making the public contract obvious at the top of every file.

End user experience
No UI change. Faster delivery and fewer regressions because is easy to audit and read the code.

GUIDELINES TBD

Order (top to bottom)

Public:@prop (public), @event, @method (public), exported types

Wiring: @watch for public props, @element, lifecycle (connectedCallback, componentWillLoad, etc.)

Internals: @listen (internal), @State, private internal props, private methods

Render: render() -> I found easier to be at the bottom, because you can easily scan visually what it implements, but would also make sense to be before internals.

Naming and visibility

Public props/events/methods: no underscore. Private fields/helpers: leading underscore or private keyword.

Events use for example sixFocusEvent and not just sixFocus

Booleans start with is/has/can.

CSS clsses starting with the same name as the component i.e. bad case -> six-button

Formatting and size

One component per file.

Keep render functions small; extract private helpers.

Do not place loose variables outside classes whilst they can be set as a static variable

Co-locate styles and tests with the component.

Enforcement

ESLint rules (maybe)

Documentation guidelines for future components

Not sure if there are plugins to enforce further. I would prefer to start with refactoring first and then think about more ways to enforce.

--

Stencil already has some guidelines https://stenciljs.com/docs/style-guide
They claim to follow Clean code guildeines but it doesnt. In their example they place 'Own Properties' first and wiring related @element on top, which makes no sense. Public API should bubble top and not the other way around.

What does the proposed API look like?

@Component({
  tag: 'six-something',
  styleUrls: {
    dark: 'six-something.dark.css',
    light: 'six-something.light.css',
  }
})
export class SixSomething {
 
  @Prop() content: string;
  @Prop() enabled: boolean;
  @Prop() menuId: string;
  @Prop() type = 'overlay';
  @Prop() swipeEnabled = true;
 
  @Event() ionCloseEvent: EventEmitter;
  @Event() ionDragEvent: EventEmitter;
  @Event() ionOpenEvent: EventEmitter;

  @Method()
  async open(): Promise<boolean> {
    // ...
    return true;
  }

  @Method()
  async close(): Promise<void> {
    // ...
  }

  @Element() el: HTMLElement;

  @Watch('swipeEnabled')
  swipeEnabledChanged(newSwipeEnabled: boolean, oldSwipeEnabled: boolean) {
    this.updateState();
  }

  @Listen('click', { enabled: false })
  onClick(ev: UIEvent) {
    console.log('hi!')
  }

  @State() isValidated: boolean;
  @State() status = 0;

  connectedCallback() {}
  disconnectedCallback() {}
  componentWillLoad() {}
  componentDidLoad() {}
  componentShouldUpdate(newVal: any, oldVal: any, propName: string) {}
  componentWillUpdate() {}
  componentDidUpdate() {}
  componentWillRender() {}
  componentDidRender() {}


  public static id = 0
  private num: number;
  private someText = 'default';

 
  private prepareAnimation(): Promise<void> {
    // ...
  }

  private updateState() {
    // ...
  }
 
  render() {
    return (
      <Host
        attribute="navigation"
        side={this.isRightSide ? 'right' : 'left'}
        type={this.type}
        class={{
          'six-something': this.isAnimating
        }}
      >
        <div class='menu-inner page-inner'>
          <slot></slot>
        </div>
      </Host>
    );
  }
}

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions