Skip to content

Coding Guidelines

Jump to bottom
Anton Kosyakov edited this page Jul 28, 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

  • 1. Do not export types or functions unless you need to share it across multiple components, see as well
  • 2. Do not introduce new types or values to the global namespace
  • 3. Always declare a return type in order to avoid accidental breaking changes because of changes to a method body.

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.

  • 7. Use LabelProvider.getLongName(uri) to get a human readable representation of a full path. Don't use uri.toString() or uri.path.toString().
  • 8. Use LabelProvider.getName(uri) to get a human readable representation of a simple file name.
  • 9. Use LabelProvider.getIcon(uri) to get a proper file icon.

Project Management

Documentation

Clone this wiki locally