Skip to content

Coding Guidelines

Jump to bottom
Akos Kitta edited this page Jul 10, 2019 · 46 revisions

Indentation

Use 4 spaces per indentation

Names

  • 1. Use PascalCase for type names
  • 2. Use PascalCase for enum values
  • 3. Use camelCase for function and method names
  • 4. Use camelCase for property names and local variables
  • 5. Use whole words in names when possible 
// bad
const termWdgId = 1;

// good
const terminalWidgetId = 1;
  • 6. Use lower case, dash-separated file names (e.g. document-provider.ts)
  • 7. Name files after the main Type it exports to easy file search by a type name
  • 8. Give unique names to types and files to avoid duplicate records in file and type search. Use specific names to achieve it.
// bad
export interface TitleButton {}

// good
export interface QuickInputTitleButton {}

  • 9. Do not use "_" as a prefix for private properties, exceptions:
    • 9.1 you want to expose a property through get/set and use underscore for the internal field
    • 9.2 you need to attach internal data to user visible JSON objects
  • 10. Names of events follow the on[Will|Did]VerbNoun? pattern. The name signals if the event is going to happen (onWill) or already happened (onDid), what happened (verb), and the context (noun) unless obvious from the context.
  • 11. Give unique names to keybinding contexts and keys to avoid collisions at runtime. Use specific names to achieve it.
// bad
export namespace TerminalSearchKeybindingContext {
    export const disableSearch = 'hideSearch';
}

// good
export namespace TerminalSearchKeybindingContext {
    export const disableSearch = 'terminalHideSearch';
}

// bad
const terminalFocusKey = this.contextKeyService.createKey<boolean>('focus', false);

// good
const terminalFocusKey = this.contextKeyService.createKey<boolean>('terminalFocus', false);

Types

  • Do not export types or functions unless you need to share it across multiple components, see as well
  • Do not introduce new types or values to the global namespace

Interfaces/Symbols

  • 1. Do not use I prefix for interfaces. Use Impl suffix for implementation of interfaces with the same name. See 624 for the discussion on this.
  • 2. Use classes instead of interfaces + symbols when possible to avoid boilerplate.
// bad
export const TaskDefinitionRegistry = Symbol('TaskDefinitionRegistry');
export interface TaskDefinitionRegistry {
    register(definition: TaskDefinition): void;
}
export class TaskDefinitionRegistryImpl implements TaskDefinitionRegistry {
    register(definition: TaskDefinition): void {
    }
}
bind(TaskDefinitionRegistryImpl).toSelf().inSingletonScope();
bind(TaskDefinitionRegistry).toService(TaskDefinitionRegistryImpl);

// good
export class TaskDefinitionRegistry {
    register(definition: TaskDefinition): void {
    }
}
bind(TaskDefinitionRegistry).toSelf().inSingletonScope();

Exceptions

  • 2.1 Remote services should be declared as an interface + a symbol in order to be used in the frontend and backend.

Comments

  • Use JSDoc style comments for functions, interfaces, enums, and classes

Strings

null and undefined

Use undefined, do not use null.

Style

  • Use arrow functions => over anonymous function expressions
  • Only surround arrow function parameters when necessary. For example, (x) => x + x is wrong but the following are correct:
x => x + x
(x,y) => x + y
<T>(x: T, y: T) => x === y
  • Always surround loop and conditional bodies with curly braces
  • Open curly braces always go on the same line as whatever necessitates them
  • Parenthesized constructs should have no surrounding whitespace. A single space follows commas, colons, and semicolons in those constructs. For example:
for (var i = 0, n = str.length; i < 10; i++) { }
if (x < 10) { }
function f(x: number, y: string): void { }
  • Use a single declaration per variable statement
    (i.e. use var x = 1; var y = 2; over var x = 1, y = 2;).
  • else goes on the line of the closing curly brace.

Dependency Injection

  • 1. Use the property injection over the construction injection. Adding new dependencies via the construction injection is a breaking change.
  • 2. Use postConstruct to initialize an object, for example to register event listeners.
@injectable()
export class MyComponent {

    @inject(ApplicationShell)
    protected readonly shell: ApplicationShell;

    @postConstruct()
    protected init(): void {
        this.shell.activeChanged.connect(() => this.doSomething());
    }

}

  • 3. Make sure to add inSingletonScope for singleton instances, otherwise a new instance will be created on each injection request.
// bad
bind(CommandContribution).to(LoggerFrontendContribution);

// good
bind(CommandContribution).to(LoggerFrontendContribution).inSingletonScope();

  • 4. Don't export functions, convert them into class methods. Functions cannot be overridden to change the behaviour or workaround a bug.
// bad
export function createWebSocket(url: string): WebSocket {
   ...  
}

// good
@injectable()
export class WebSocketProvider {
   protected createWebSocket(url: string): WebSocket {
       ...
   }
}

@injectable()
export class MyWebSocketProvider extends WebSocketProvider {
   protected createWebSocket(url: string): WebSocket {
      // create a web socket with custom options
   }
}

Exceptions

  • 4.1 Convenient functions which are based on the stable API can be exported in the corresponding namespace.

In this case clients:

  • can customize behaviour via exchanging the API implementation
  • have a choice to use convenient functions or an API directly
export namespace MonacoEditor {
    // convenient function to get a Monaco editor based on the editor manager API
    export function getCurrent(manager: EditorManager): MonacoEditor | undefined {
        return get(manager.currentEditor);
    }
    ...
}

  • 4.2 The special case of 4.1 is functions on a JSON type.

JSON types are not supposed to be implementable, but only instantiable. They cannot have functions to avoid serialization issues.

export interface CompositeTreeNode extends TreeNode {
    children: ReadonlyArray<TreeNode>;

    // bad - JSON types should not have functions
    getFirstChild(): TreeNode | undefined;
}

// good - JSON types can have corresponding namespaces with functions
export namespace CompositeTreeNode {
    export function getFirstChild(parent: CompositeTreeNode): TreeNode | undefined {
        return parent.children[0];
    }
    ...
}

// bad - JSON types should not be implemented
export class MyCompositeTreeNode implements CompositeTreeNode {
    ...
}

// good - JSON types can be extended
export interface MyCompositeTreeNode extends CompositeTreeNode {
    ...
}

  • 4.3 Auxiliary functions which are called from the customizable context can be exported in the corresponding namespace.
@injectable()
export class DirtyDiffModel {
    // this method can be overridden, subclasses have an access to `DirtyDiffModel.documentContentLines`
    protected handleDocumentChanged(document: TextEditorDocument): void {
        this.currentContent = DirtyDiffModel.documentContentLines(document);
        this.update();
    }
}
export namespace DirtyDiffModel {
    // the auxiliary function
    export function documentContentLines(document: TextEditorDocument): ContentLines {
        ...
    }
}

CSS

  • Use the lower-case-with-dashes format.
  • Prefix classes with theia when used as global classes.

React

  • 1. Do not bind functions in event handlers.
    • Extract a react component if you want to pass state to an event handler function.

Why? Because it creates a new instance of event handler function on each render and breaks React element caching leading to rerendering and bad performance.

// bad
class MyWidget extends ReactWidget {
  render(): React.ReactNode {
    return <div onClick={this.onClickDiv.bind(this)} />;
  }

  protected onClickDiv(): void {
    // do stuff
  }
}

// bad
class MyWidget extends ReactWidget {
  render(): React.ReactNode {
    return <div onClick={() => this.onClickDiv()} />;
  }

  protected onClickDiv(): void {
    // do stuff
  }
}

// very bad
class MyWidget extends ReactWidget {
  render(): React.ReactNode {
    return <div onClick={this.onClickDiv} />;
  }

  protected onClickDiv(): void {
    // do stuff, no `this` access
  }
}

// good
class MyWidget extends ReactWidget {
  render(): React.ReactNode {
    return <div onClick={this.onClickDiv} />
  }

  protected onClickDiv = () => {
    // do stuff, can access `this`
  }
}

URI/Path

  • 1. Pass URIs between frontend and backend, never paths.

Why? Frontend and backend can have different operating systems leading to incompatibilities between paths. URIs are normalized in order to be os-agnostic.

  • 2. Use FileSystem.getFsPath to get a path on the frontend from a URI.
  • 3. Use FileUri.fsPath to get a path on the backend from a URI. Never use it on the frontend.
  • 4. Always define an explicit scheme for a URI.

Why? A URI without scheme will fall back to file scheme for now, in the future it will lead to a runtime error.

  • 5. Use Path Theia API to manipulate paths on the frontend. Don't use Node.js APIs like path module. Also see the code organization guideline.
  • 6. On the backend, prefer Node.js APIS to manipulate the file system, like fs and fs-extra modules, over FileSystem Theia API.

Why? FileSystem is a service to expose file system capabilities to the frontend. It's aligned with expectations and requirements on the frontend. Using it on the backend is overkill in the most cases.

Project Management

Documentation

Clone this wiki locally