Coding guidelines

General

• Code comments should augment the code itself.
  • Explain why it works this way, instead of what the code does. Leave yourself a reminder that'll be useful in six months.
• Commit messages should be descriptive and useful to anyone at any point in time.
  • Avoid "respond to comments" which only makes sense in the immediate context of the PR, prefer "style fixes + better naming" which is accurate and helpful.
  • Don't be afraid to split a change into multiple commits (or even multiple PRs) if it promotes readability and would ease the review burden.
• Squash PRs into a single commit on the main branch. We use GitHub's "Squash & Merge" feature to do this automatically when merging PRs.
• TODOs should generally not appear in committed code. If that is completely unavoidable then create a GitHub issue to track resolving the TODO. If it is a hack with a clear plan for paying down the technical debt in the future, mark it as a // HACKHACK <with explanation> instead ("HACKHACK" token used easy searchability).

Sass

• All class names begin with a namespace that should be unique. This is a simple principle to avoid naming collisions.
  • Blueprint's namespace looks like bp#- where # is the current major version.
• Source files should be named as _snake-case.scss. Note the leading underscore: this is a Sass convention to denote a partial file that will be imported by another file. Each project has one non-underscore-prefixed "root" file.
• Sass variables should be named according to the following pattern: $[theme?]-[component]-[property]-[variant?].
  • [theme] is typically dark or omitted.
  • [component] is the name of component to which the variable applies, such as button or input-group.
  • [property] is the CSS property the variable should be used for and a hint to the type of the variable, such as height or background-color.
  • [variant] is an optional modifier such as large or hover where the variable needs a different value. If a variable exists with a -variant then one without a variant should exist as well (for instance, $pt-font-size and $pt-font-size-large).
• We rely heavily on Stylelint to ensure consistent formatting across our stylesheets.
• We use a slightly modified version of Concentric CSS for property ordering with tweaks made to better suit the box-sizing: border-box model. See the full configuration in stylelint-config-palantir.
• "Public" variables exposed in the published lib/{less,scsss}/variables.{less,scss} should have the pt- prefix, while local component variables should not.
• Prefer @mixins() over %placeholders because the latter will emit entire duplicated selectors when imported into multiple files. While mixins emit the same properties in multiple places, they provide reliable control over exactly which CSS is emitted where.
• Prefer longer unambiguous class names over nesting. This results in lower specificity and much less reliance on a specific DOM structure.

CSS selectors

Which selectors are OK? Which are too slow? Which cause problems in the long run?

• The worst thing you can do for CSS performance and maintainability is mimic the HTML structure.
  • Instead, use namespaced unambiguous class names that are specific enough to target an element with minimal nesting. In other words, prefer longer class names over selector specificity.
• Avoid universal selectors (those without a tag, class, or ID) if there's any other option.
• Avoid qualifying classes with a tag, like span.pt-icon-standard. This ties your rule to HTML structure and makes it less flexible for the user.
• Avoid direct descendant > selectors. This ties your style to your markup and reduces fault tolerance. For example, adding a wrapper layer between A and B in .a > .b will break the styles.
  • > selectors can still be useful in specific scenarios such as context-specific overrides (like the main icon in a Callout). Just avoid over-reliance.
• Brush up on some CSS tips from Mozilla

TypeScript

• Prefer explicit, static language constructs over implicit, dynamic ones. This makes the code base easier to approach for newcomers. Generally follow the advice of JavaScript: The Good Parts.

• Avoid implicit type coercion, particularly when testing existence. The strict-boolean-expressions lint rule helps with this.

  // bad
  if (x) { ... }
  // good
  if (x != null) { ... }

• Leverage the type system to push as many possible programming errors as possible from runtime to compile time. TypeScript is not strongly-typed (rather, it is a gradually typed language with very high interoperability with regular JavaScript), so you can't go all the way with this. You will still have to rely on good unit test coverage to verify your code.

  • Use the --noImplicitAny compiler option.
  • Should go without saying, but try to avoid casting to any.

• Become familiar with type inference. Leverage it to make your code more expressive and readable while retaining type safety.

• Having private fields signifies the strongest intent of encapsulation, and should be the first modifier to consider when writing a new method or field. It is fine to make them public if they're either used outside the class or if they're absolutely necessary for tests.

• Public interfaces must have JSDoc comments.

• Source files should be camelCased.ts.

• Prefer user-defined type guards instead of explicit type assertions (e.g. someValue as SomeType). While these constructs are similar in strictness, type guards are easier to read. See the TSLint rule suggestion.

• Our source files are written as modules in various folders. Exports from these modules are funneled through index.ts, which re-exports the public API of a package (for example, see core/src/index.ts).

  • Any entities which are not funneled through this entry point but are exported for usage across modules (sometimes just for testing) should be marked as /** @internal */ to elide them from the generated typings.
  • This makes it easy to tell at a glance what is part of the public API and what isn't.

React

• Leverage the type system to check your JSX as much as possible. Prefer defining stricter props interfaces over runtime validation of props. Do not use React's built-in runtime type checking mechanism of propTypes — it is redundant in TypeScript.

• Bind component class methods upon declaration, not in the render() method. Using dynamically-bound handlers is less performant because it will cause the React reconciler to trigger more re-renders.

  class Button {
    public render() {
      return <button onClick={this.handleClick} />;
    }
  
    private handleClick = (e: React.SyntheticEvent) => {
      ...
    }
  }

• In general, extend React.PureComponent to improve performance of components.

• Use callback-based refs on components instead of string-based refs.

  • React is deprecating string-based refs soon.
  • Callback-based refs are strongly typed. See this blog post.
  • Enforced by the tslint-react rule jsx-no-string-ref.

Browser compatibility tips

• Avoid someDomElement.children and instead use someDomElement.childNodes. SVG elements in IE don't have a children property, so trying to access it will cause a runtime error.