Coding guidelines

General

• Code comments should attempt to explain why a change was made, instead of what the change is. Let the code speak for itself.
• 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.
• 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, use HACKHACK instead (for easier searchability).

Sass

• All class names begin with pt-. This is a simple trick for avoiding naming collisions.
• 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 a 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 the stylelint-config-palantir repo.
• "Public" variables exposed in variables.scss (via the "sass-variables" gulp task) should have the pt- prefix, while local variables should not.
• Prefer limited usage of Sass's language features. Stick closer to regular CSS. Many Sass features are overly expressive and complicate the language, making it difficult to debug code architecture or performance issues.
  • Our most frequently used features (by a longshot, in order) are variables, mixins and nesting.
  • We've run into a number of issues with placeholders, most significantly when circumventing the PlaceholderInExtend scss-lint rule. As a result we try to use mixins instead of placeholders.

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 clear namespaced class names that are specific enough to target an element with minimal nesting.
• 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.
• Direct descendant > selectors make your HTML markup more strict. It reduces flexibility of the styles. Often this hurts consumers when they want to add a wrapper layer between A and B in a .class-a > .class-b. Try to avoid them unless the alternative would make the API extremely verbose.
• 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.

  // 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 (either the <> or as syntax). 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. The latter approach is less performant and results in greater garbage collection than is necessary.

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

• Use the @PureRender decorator 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.

References & further reading

• Swift API design guidelines