Skip to content

Coding Guidelines

Jump to bottom
Mark Sujew edited this page Nov 30, 2021 · 46 revisions

Indentation

Use 4 spaces per indentation

Imports

Use organize imports to sort imports, but make sure the imports work properly (i.e. imports from /src/ for *.ts files are bad but sometimes inserted).

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.

Why? It should be easy to find a type by a file name.

  • 7.1 Avoid one file with many large classes, put each class in own file.

Why? It should be easy to find a class by a file name.

  • 8. Give unique names to types and files. Use specific names to achieve it.

Why? In order to avoid duplicate records in file and type search.

// 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.

Internationalization/Localization

  • 1. Always localize user facing text with the nls.localize(key, defaultValue, ...args) function.

What is user facing text? Any strings that are hardcoded (not calculated) that could be in any way visible to the user, be it labels for commands and menus, messages/notifications/dialogs, quick input placeholders or preferences.

  • 1.1. Parameters for messages should be passed as the args of the localize function. They are inserted at the location of the placeholders - in the form of {\d+} - in the localized text. E.g. {0} will be replaced with the first arg, {1} with the second, etc.
// bad
nls.localize('hello', `Hello there ${name}.`);

// good
nls.localize('hello', 'Hello there {0}.', name);

  • 1.2. The nls.localizeByDefault function automatically finds the translation key for vscode's language packs just by using the default value as its argument and translates it into the currently used locale. If the nls.localizeByDefault function is not able to find a key for the supplied default value, a warning will be shown in the browser console. When there's no appropriate translation in vscode, just use the nls.localize function with a new key using the syntax theia/<package>/<id>.
// bad
nls.localize('vscode/dialogService/close', 'Close');

// good
nls.localizeByDefault('Close');

  • 2. Use utility functions where possible:
// bad
command: Command = { label: nls.localize(key, defaultValue), originalLabel: defaultValue };

// good
command = Command.toLocalizedCommand({ id: key, label: defaultValue });

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 {
        ...
    }
}

  • 5. Don't use multi inject, use ContributionProvider to inject multiple instances.

Why?

  • ContributionProvider is a documented way to introduce contribution points. See Contribution-Points: https://www.theia-ide.org/docs/services_and_contributions
  • If there is no a single binding, multi inject resolves to undefined, not an empty array. ContributionProvider provides an empty array.
  • Multi inject does not guarantee the same instances are injected if an extender does not use inSingletonScope. ContributionProvider caches instances to ensure uniqueness.

CSS

  • 1. Use the lower-case-with-dashes format.
  • 2. Prefix classes with theia when used as global classes.
  • 3. Do not define styles in code. Introduce proper CSS classes.

Why? It is not possible to play with such styles in the dev tools without recompiling the code. CSS classes can be edited in the dev tools.

Theming

  • 1. Do not introduce css color variables. Implement ColorContribution and use ColorRegistry.register to register new colors.
  • 2. Do not introduce hardcode color values in css. Instead reference VS Code colors in css by prefixing them with --theia and replacing all dots with dashes. For example widget.shadow color can be referenced in css with var(--theia-widget-shadow).
  • 3. Always derive new colors from existing VS Code colors. One can derive from an existing color, just by plainly referencing it, e.g. dark: 'widget.shadow', or applying transformations, e.g. dark: Color.lighten('widget.shadow', 0.4).

Why? Otherwise, there is no guarantee that new colors don't look alien for a random VS Code color theme. 

// bad
'button.secondary.foreground'
'button.secondary.disabled.foreground'

// good
'secondaryButton.foregrond'
'secondaryButton.disabledForegrond'

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. URIs should be sent as strings in JSON-RPC services, e.g. RemoteFileSystemServer accepts strings, not URIs.

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 FileService.fsPath 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, use Node.js APIS to manipulate the file system, like fs and fs-extra modules.

Why? FileService is to expose file system capabilities to the frontend only. It's aligned with expectations and requirements on the frontend. Using it on the backend is not possible.

  • 7. Use LabelProvider.getLongName(uri) to get a systemwide human readable representation of a full path. Don't use uri.toString() or uri.path.toString().
  • 8. Use LabelProvider.getName(uri) to get a systemwide human readable representation of a simple file name.
  • 9. Use LabelProvider.getIcon(uri) to get a systemwide file icon.
  • 10. Don't use string to manipulate URIs and paths. Use URI and Path capabilities instead, like join, resolve and relative.

Why? Because object representation can handle corner cases properly, like trailing separators.

// bad
uriString + '/' + pathString

// good
new URI(uriString).join(pathString)

// bad
pathString.substring(absolutePathString.length + 1)

// good
new Path(absolutePathString).relative(pathString)

Logging

  • 1. Use console instead of Ilogger for the root (top-level) logging.
// bad
@inject(ILogger)
protected readonly logger: ILogger;

this.logger.info(``);

// good
console.info(``)

Why? All calls to console are intercepted on the frontend and backend, and then forwarded to ILogger already. Log level can be configured by theia start --log-level=debug.

Project Management

Documentation

Clone this wiki locally