Skip to content

Code conventions

Jump to bottom
Christopher Joel edited this page Oct 31, 2019 · 17 revisions

Tooling-enforced conventions

<model-viewer> strives to be consistent with commonly accepted code conventions and formatting best practices. To this end, we have documented many of our conventions as tooling configuration. Currently, we use the following tools to enforce some formating and style rules:

  • clang-format
  • tsc (the TypeScript compiler)

We are also looking into adding tslint to this list. If you know of tools that we can use to automate checks anything that is considered conventional, please let us know! We are eager to document as much of our conventions as possible with tools.

Please ensure that you have your local environment set up to take full advantage of the tools above! Many of us use plugins (in Vim and VSCode) to auto-run these tools as we are developing. There is a good chance that your editor of choice has such plug-ins available as well.

Project-specific guidelines

There are some code conventions that are project-specific and not yet enforced with tools, or not easily enforced with tools. To the extent possible, we would like to enforce all conventions with tools, but until we can we will document them here for every contributor's reference.

General

  • All new library code should be authored in TypeScript
    • If you do work on a JavaScript file, please consider helping us by converting it to TypeScript as you go!
  • Always use const/let when declaring variables; prefer const where possible
  • Local modules must always be imported with bare path specifiers, including their .js extension
  • External modules (e.g., those installed with npm) must always be imported with name specifiers

Naming

Naming things is one of the great unsolved problems of computer science. Here are some tips to help you along the path of existential uncertainty:

  • A good name is as concise as possible, while also being completely descriptive with minimal context
  • Use full words for identifiers, not abbreviations
    • e could mean error or event. So, just write error or event!
  • Use static constants in place of magic numbers or singleton values
    • e.g., const KERNELS_SECRET_CHICKEN_FRY_OVEN_RATIO = 1.618;

Tests

You should write tests related to any change where:

  • More code was added than was removed (usually)
  • Other tests were removed (usually)
  • A bug was fixed (always)

If you did not add new tests in a change, please include a comprehensive explanation for why no new tests were required!

API access

Standard JavaScript has no first-class mechanism for scoping API access. So, to the extent that it is ever done, it is always enforced as a convention. This section documents our project's convention.

API in <model-viewer> has three meaningful access levels:

  • public: can be invoked by end-users of the library
  • private: strictly internal to the class, mixin or module that implements it
  • protected/shared: accessible by subclasses, importing modules and tests

Some of our code predates these conventions. Particularly, Three.js "components" that have not been converted to TypeScript may not adhere to a reliable distinction between different levels of API access. Any APIs that have been converted to TypeScript are implicitly public unless declared otherwise.

Public API

Public API is API that fits any of the following criteria:

  • It is a statically assigned export of a module
  • It is considered public API according to the LitElement documentation
  • It is a bare property defined on a class or object, and not considered private/protected API according to the LitElement documentation

Private API

Private API must:

  • Use the private TypeScript access modifier where applicable
  • Be assigned to a JavaScript Symbol computed property
  • Assign its Symbol to an identifier that begins with $
  • Not directly export the Symbol from the module where it is created

For example:

const $privateMethod = Symbol('privateMethod');

export class CoolClass {
  private [$privateMethod]() {
    // Private implementation here
  }
}

Protected/shared API

For protected/shared API, all of the rules for private API apply except:

  • It uses the protected TypeScript access modifier where applicable
  • The Symbol must be directly exported from the module where it is created

For example:

export const $protectedMethod = Symbol('protectedMethod');

export class CoolClass {
  protected [$protectedMethod]() {
    // Shared implementation here
  }
}

Documentation & comments

We assume that code will be read many more times than it is written or modified, and we optimize for legibility.

Always add documentation to your code in the form of comments for:

  • New public API (as defined in the section above)
  • Recording future changes that should be made in part of the code
  • Any code that pertains to quirky behavior across browsers, GPUs etc.
  • Code whose purpose may not be dead obvious to a casual reader

Notes

If you consider yourself a knowledgeable person in the domain of some potentially confusing part of the code, leave a "note" for future readers that contains your Github username or other well-known personal identifier. For example:

// NOTE(cdata): This code be crazy for reasons

Todos

If any code that is proposed requires known future work, the following steps should always be taken:

  1. File an issue to describe the future work
  2. Leave a "todo" comment near the code

A "todo" comment should always reference the number of the issue that has been filed. For example:

// TODO(#123): Convert this whole library to Rust ASAP

External references

If there are external documents or resources that will help a reader further investigate the topic of the comment, add an @see JSDoc annotation:

// Unicorns are beautiful
// @see http://cornify.com

License headers

With a few exceptions, all source code should contain the appropriate license header at the top. Please ensure that the current year is used when adding a license header to a new source file!

JavaScript/TypeScript/CSS license header

For JavaScript/TypeScript/CSS, the license header looks like this:

/* @license
 * Copyright 2019 Google LLC. All Rights Reserved.
 * Licensed under the Apache License, Version 2.0 (the 'License');
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an 'AS IS' BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

HTML license header

For HTML, the license header should look like this:

<!--
/* @license
 * Copyright 2019 Google LLC. All Rights Reserved.
 * Licensed under the Apache License, Version 2.0 (the 'License');
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an 'AS IS' BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-->

Shell license header

For shell scripts, the license header should look like this:

##
# Copyright 2019 Google LLC. All Rights Reserved.
# Licensed under the Apache License, Version 2.0 (the 'License');
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an 'AS IS' BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
##

Shared implementation

We take advantage of a dynamic inheritance pattern that is somewhat unique to JavaScript in order to break our implementation up into a series of independent mixins. A basic example of this pattern in plain JavaScript looks like this:

// In cool-mixin.js:
export const CoolMixin = (SuperClass) => class extends SuperClass {
  coolMixinMethod() {
    console.log('World');
  }
};

// In another module:
import {CoolMixin} from './cool-mixin.js';

class BaseClass {
  constructor() {
    console.log('Hello');
  }
}

class CoolClass extends CoolMixin(BaseClass) {}

const cool = new CoolClass();
cool.coolMixinMethod();

Wherever possible, API that decorates the Custom Element implementation of <model-viewer> should be a part of a mixin that decorates the ModelViewerBaseElement class.

For cases where API needs to be shared between mixins, it should happen in one of the following ways:

  • The shared implementation should manifest as CustomEvents dispatched from the element
    • Note that events dispatched on the element always constitute public API
  • The implementation should be added to ModelViewerBaseElement

Module exports

When it comes to JavaScript's module system, there are two different kinds of module exports: named and default. You can read more about JavaScript module exports on MDN.

All modules should use named exports unless there is a really good reason not to.

Visible UI elements

Whenever we add visible UI elements to <model-viewer>, they automatically inherit the use case that they should be customizable or removable by users.

For all UI elements, users should always have the following two options:

  1. Customize built-in UI styles (to the extent that it makes sense) via CSS custom properties (and someday also CSS shadow parts)
  2. Bring your own UI via Shadow DOM content projection

You can see an example of Option 1 in the PR description of https://github.com/GoogleWebComponents/model-viewer/pull/487 where we added CSS custom properties for styling the progress bar and poster.

With Option 1, we make it easy to rebrand the built-in UI by customizing details like color, font face, icons and perhaps paddings, margins and font sizes in some cases.

You can see an example of Option 2 in the PR description of https://github.com/GoogleWebComponents/model-viewer/pull/322 where we added the interaction prompt.

Option 2 is a more robust approach to customization. It allows the content author to provide complex, bespoke alternatives to the built-in UI.

Clone this wiki locally