-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Coding guidelines
Antoine Llorca edited this page Nov 10, 2016
·
11 revisions
- 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.
- A TODO should generally not be committed but if it is completely unavoidable it should be accompanied by a Github issue to track resolving the TODO. If it is hack, use HACKHACK instead (for easier searchability).
- 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 typicallydarkor omitted. -
[component]is the name of component to which the variable applies, such asbuttonorinput-group. -
[property]is the CSS property the variable should be used for and a hint to the type of the variable, such asheightorbackground-color. -
[variant]is a modifier such aslargeorhoverwhere the variable needs a different value. if a variable exists with a-variantthen one without a variant should exist as well (for instance,$pt-font-sizeand$pt-font-size-large).
-
- A lot of style rules are enforced by scss-lint, so it would help to familiarize yourself with the available linters.
- We use a slightly modified version of Concentric CSS for property ordering with tweaks made to better suit the
box-sizing: border-boxmodel. See the full configuration in our external stylelint-config-palantir repo. - Variables exported to
variables.{scss,less}(via the "sass-variables" gulp task) should have thept-prefix, while locally used 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
PlaceholderInExtendscss-lint rule. We're trying to reduce usage of placeholders in general because they can be difficult to reason about.
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
- 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. Some examples:
// bad opacityPercentage = +element.style.opacity * 100; // good opacityPercentage = parseInt(element.style.opacity, 10) * 100; // bad if (x) { ... } // good if (x != null) { ... }
- Avoid indexing into objects with strings. Instead, use strongly-typed property access.
- Note that in TypeScript 2.0, objects will get implicit index signatures, which makes this a little harder to enforce.
- Avoid implicit type coercion. Some examples:
- 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
--noImplicitAnycompiler option. - Should go without saying, but try to avoid casting to
<any>.
- Use the
- 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 should have JSDoc comments.
- Source files should be
camelCased.ts. - Prefer user-defined type guards instead of explicit type assertions (either the
<>orassyntax). While these constructs are similar in strictness, type guards are easier to read. TSLint rule suggestion here. - 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, seeblueprint-components/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.
-
Any entities which are not funneled through this entry point but are
- 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
@PureRenderdecorator 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.
- Avoid
someDomElement.childrenand instead usesomeDomElement.childNodes. SVG elements in IE don't have achildrenproperty, so trying to access it will cause a runtime error.
- FAQ
- 6.x Changelog
- 5.x Changelog
- 5.0 pre-release changelog
- 4.x Changelog
- v4.0 & v5.0 major version semantic swap
- v6.0 changes
- Spacing System Migration: 10px to 4px
- react-day-picker v8 migration
- HotkeysTarget & useHotkeys migration
- PanelStack2 migration
- Table 6.0 changes