anpigon's picture
Add favicon and image assets for Obsidian help and developer documentation
c63ff03
|
raw
history blame
11 kB

This page lists common review comments plugin authors get when submitting their plugin.

While the guidelines on this page are recommendations, depending on their severity, we may still require you to address any violations.

Policies for plugin developers Make sure that you've read our [[Developer policies]] as well as the [[Submission requirements for plugins]].

General

Avoid using global app instance

Avoid using the global app object, app (or window.app). Instead, use the reference provided by your plugin instance, this.app.

The global app object is intended for debugging purposes and might be removed in the future.

UI text

This section lists guidelines for formatting text in the user interface, such as settings, commands, and buttons.

The example below from Settings → Appearance demonstrates the guidelines for text in the user interface.

![[settings-headings.png]]

  1. [[#Only use headings under settings if you have more than one section.|General settings are at the top and don't have a heading]].
  2. [[#Avoid "settings" in settings headings|Section headings don't have "settings" in the heading text]].
  3. [[#Use Sentence case in UI]].

For more information on writing and formatting text for Obsidian, refer to our Style guide.

Only use headings under settings if you have more than one section.

Avoid adding a top-level heading in the settings tab, such as "General", "Settings", or the name of your plugin.

If you have more than one section under settings, and one contains general settings, keep them at the top without adding a heading.

For example, look at the settings under Settings → Appearance:

Avoid "settings" in settings headings

In the settings tab, you can add headings to organize settings. Avoid including the word "settings" to these headings. Since everything in under the settings tab is settings, repeating it for every heading becomes redundant.

  • Prefer "Advanced" over "Advanced settings".
  • Prefer "Templates" over "Settings for templates".

Use sentence case in UI

Any text in UI elements should be using Sentence case instead of Title Case, where only the first word in a sentence, and proper nouns, should be capitalized.

  • Prefer "Template folder location" over "Template Folder Location".
  • Prefer "Create new note" over "Create New Note".

Security

Avoid innerHTML, outerHTML and insertAdjacentHTML

Building DOM elements from user-defined input, using innerHTML, outerHTML and insertAdjacentHTML can pose a security risk.

The following example builds a DOM element using a string that contains user input, ${name}. name can contain other DOM elements, such as <script>alert()</script>, and can allow a potential attacker to execute arbitrary code on the user's computer.

function showName(name: string) {
  let containerElement = document.querySelector('.my-container');
  // DON'T DO THIS
  containerElement.innerHTML = `<div class="my-class"><b>Your name is: </b>${name}</div>`;
}

Instead, use the DOM API or the Obsidian helper functions, such as createEl(), createDiv() and createSpan() to build the DOM element programmatically. For more information, refer to [[HTML elements]].

Resource management

Clean up resources when plugin unloads

Any resources created by the plugin, such as event listeners, must be destroyed or released when the plugin unloads.

When possible, use methods like [[registerEvent|registerEvent()]] or [[addCommand|addCommand()]] to automatically clean up resources when the plugin unloads.

export default class MyPlugin extends Plugin {
  onload() {
    this.registerEvent(this.app.vault.on("create", this.onCreate));
  }

  onCreate: (file: TAbstractFile) => {
    // ...
  }
}

You don't need to clean up resources that are guaranteed to be removed when your plugin unloads. For example, if you register a mouseenter listener on a DOM element, the event listener will be garbage-collected when the element goes out of scope.

Don't detach leaves in onunload

When the user updates your plugin, any open leaves will be reinitialized at their original position, regardless of where the user had moved them.

Commands

Avoid setting a default hotkey for commands

Setting a default hotkey may lead to conflicts between plugins and may override hotkeys that the user has already configured.

It's also difficult to choose a default hotkey that is available on all operating systems.

Use the appropriate callback type for commands

When you add a command in your plugin, use the appropriate callback type.

  • Use callback if the command runs unconditionally.
  • Use checkCallback if the command only runs under certain conditions.

If the command requires an open and active Markdown editor, use editorCallback, or the corresponding editorCheckCallback.

Workspace

Avoid accessing workspace.activeLeaf directly

If you want to access the active view, use [[getActiveViewOfType|getActiveViewOfType()]] instead:

const view = this.app.workspace.getActiveViewOfType(MarkdownView);

// getActiveViewOfType will return null if the active view is null, or if it's not a MarkdownView.
if (view) {
  // ...
}

If you want to access the editor in the active note, use activeEditor instead:

const editor = this.app.workspace.activeEditor;

Avoid managing references to custom views

Managing references to custom view can cause memory leaks or unintended consequences.

Don't do this:

this.registerViewType(MY_VIEW_TYPE, () => this.view = new MyCustomView());

Do this instead:

this.registerViewType(MY_VIEW_TYPE, () => new MyCustomView());

To access the view from your plugin, use Workspace.getActiveLeavesOfType():

for (let leaf of app.workspace.getActiveLeavesOfType(MY_VIEW_TYPE)) {
  let view = leaf.view;
  if (view instanceof MyCustomView) {
    // ...
  }
}

Vault

Prefer the Editor API instead of Vault.modify

If you want to edit an active note, use the [[Editor]] interface instead of [[Vault/modify|Vault.modify()]].

Editor maintains information about the active note, such as cursor position, selection, and folded content. When you use [[Vault/modify|Vault.modify()]] to edit the note, all that information is lost, which leads to a poor experience for the user.

Editor is also more efficient when making small changes to parts of the note.

Only use [[Vault/modify|Vault.modify()]] if you're editing a file in the background.

Prefer the Vault API over the Adapter API

Obsidian exposes two APIs for file operations: the Vault API (app.vault) and the Adapter API (app.vault.adapter).

While the file operations in the Adapter API are often more familiar to many developers, the Vault API has two main advantages over the adapter.

  • Performance: The Vault API has a caching layer that can speed up file reads when the file is already known to Obsidian.
  • Safety: The Vault API performs file operations serially to avoid any race conditions, for example when reading a file that is being written to at the same time.

Avoid iterating all files to find a file by its path

This is inefficient, especially for large vaults. Use [[Vault/getAbstractFileByPath|getAbstractFileByPath()]] instead.

Don't do this:

vault.getAllFiles().find(file => file.path === filePath)

Do this instead:

const filePath = 'folder/file.md';

const file = app.vault.getAbstractFileByPath(filePath);

// Check if it exists and is of the correct type
if (file instanceof TFile) {
  // file is automatically casted to TFile within this scope.
}

Use normalizePath() to clean up user-defined paths

Use [[normalizePath|normalizePath()]] whenever you accept user-defined paths to files or folders in the vault, or when you construct your own paths in the plugin code.

normalizePath() takes a path and scrubs it to be safe for the file system and for cross-platform use. This function:

  • Cleans up the use of forward and backward slashes, such as replacing 1 or more of \ or / with a single /.
  • Removes leading and trailing forward and backward slashes.
  • Replaces any non-breaking spaces, \u00A0, with a regular space.
  • Runs the path through String.prototype.normalize.
import { normalizePath } from "obsidian";
const pathToPlugin = normalizePath(app.vault.configDir + "//plugins/my-plugin");
// pathToPlugin contains ".obsidian/plugins/my-plugin" not .obsidian//plugins/my-plugin

Editor

Change or reconfigure editor extensions

If you want to change or reconfigure an [[Editor extensions|editor extension]] after you've registered using [[registerEditorExtension|registerEditorExtension()]], use [[updateOptions|updateOptions()]] to update all editors.

class MyPlugin extends Plugin {
  private editorExtension: Extension[] = [];

  onload() {
    //...

    this.registerEditorExtension(this.editorExtension);
  }

  updateEditorExtension() {
    // Empty the array while keeping the same reference
    // (Don't create a new array here)
    this.editorExtension.length = 0;

    // Create new editor extension
    let myNewExtension = this.createEditorExtension();
    // Add it to the array
    this.editorExtension.push(myNewExtension);

    // Flush the changes to all editors
    this.app.workspace.updateOptions();
  }
}

TypeScript

Prefer const and let over var

For more information, refer to 4 Reasons Why var is Considered Obsolete in Modern JavaScript.

Prefer async/await over Promise

Recent versions of JavaScript and TypeScript support the async and await keywords to run code asynchronously, which allow for more readable code than using Promises.

Don't do this:

function test(): Promise<string | null> {
  return requestUrl('https://example.com')
    .then(res => res.text
    .catch(e => {
      console.log(e);
      return null;
    });
}

Do this instead:

async function AsyncTest(): Promise<string | null> {
  try {
    let res = await requestUrl('https://example.com');
    let text = await r.text;
    return text;
  }
  catch (e) {
    console.log(e);
    return null;
  }
}

Consider organizing your code base using folders

If your plugin uses more than one .ts file, consider organizing them into folders to make it easier to review and maintain.

Rename placeholder class names

The sample plugin contains placeholder names for common classes, such as MyPlugin, MyPluginSettings, and SampleSettingTab. Rename these to reflect the name of your plugin.