Write down some unwritten rules of Flutter development.

2026-03-30 11:09:55 -07:00 · 2015-10-09 11:08:13 -07:00
parent 63b453c9d2
commit 5afa5dec55
4 changed files with 196 additions and 45 deletions
@@ -114,6 +114,11 @@ To send us a pull request:
 * `git pull-request` (if you are using [Hub](http://github.com/github/hub/)) or go to `https://github.com/<your_name_here>/sky_engine` and click the
   "Compare & pull request" button

+Please peruse our [style guides](sky/specs/style-guide.md) and
+[design principles](sky/specs/design.md) before working on anything
+non-trivial. These guidelines are intended to keep the code consistent
+and avoid common pitfalls.
+
 Please make sure all your checkins have detailed commit messages explaining the patch.
 If you made multiple commits for a single pull request, either make sure each one has a detailed
 message explaining that specific commit, or squash your commits into one single checkin with a
@@ -0,0 +1,12 @@
+Small examples of the Flutter widget framework
+==============================================
+
+To run these, open a terminal in this directory and use the following
+command:
+
+```bash
+pub global activate flutter
+flutter start --checked -t foo.dart
+```
+
+...where `foo.dart` is the file you want to run.
@@ -1,35 +1,106 @@
 Design Principles
 =================

+Flutter is written based on some core principles that were mostly
+intuited from past experiences with other platforms such as the Web
+and Android, some of which are summarised below.
+
+Lazy programming
+----------------
+
+Write what you need and no more, but when you write it, do it right.
+
+Avoid implementing features you don't need. You can't design a feature
+without knowing what the constraints are. Implementing features "for
+completeness" results in unused code that is expensive to maintain,
+learn about, document, test, etc.
+
+When you do implement a feature, implement it the right way. Avoid
+workarounds. Workarounds merely kick the problem further down the
+road, but at a higher cost: someone will have to relearn the problem,
+figure out the workaround and how to dismantle it (and all the places
+that now use it), _and_ implement the feature.
+
+Tests
+-----
+
+When you fix a bug, first write a test that fails, then fix the bug
+and verify the test passes.
+
+When you implement a new feature, write tests for it.
+
+Run the tests before checking code in. (Travis does this for you, so
+wait for Travis to give the green light before merging a PR.)
+
+API design
+----------
+
 * There should be no objects that represent live state that reflects
  some other state, since they are expensive to maintain. e.g. no
  HTMLCollection.

-* Property getters should be efficient. If an operation is inefficient
-  it should be a method instead. e.g. document.getForms(), not
+* Property getters should be efficient (e.g. just returning a cached
+  value, or an O(1) table lookup). If an operation is inefficient it
+  should be a method instead. e.g. document.getForms(), not
  document.forms.

-* There should be no APIs that require synchronously computing layout
-  (or other expensive operations).
+* There should be no APIs that require synchronously completing an
+  expensive operation (e.g. computing a full app layout outside of the
+  layout phase).

-* Any API that can be implemented in terms of another is a convenience
-  API and should be implemented in a framework, not as part of the
-  core. e.g., no document.forms.
+* We use a layered framework design, where each layer addresses a
+  narrowly scoped problem and is then used by the next layer to solve
+  a bigger problem. This is true both at a high level (widgets relies
+  on rendering relies on painting) and at the level of individual
+  classes and methods (e.g. in the rendering library, having one class
+  for clipping and one class for opacity rather than one class that
+  does both at the same time).

- - having APIs for performance reasons is fine (e.g. querySelector()
-   could be implemented by crawling but it would be so much faster if
-   it could use the runtime's ID hashtables that it's ok to support
-   natively)
+ - Convenience APIs belong at the layer above the one they are
+   simplifying.
+
+ - Having dedicated APIs for performance reasons is fine. If one
+   specific operation, say clipping a rounded rectangle, is expensive
+   using the generic API but could be implemented more efficiently
+   using a dedicated API, then a dedicated API is fine.

 * APIs that encourage bad practices should not exist. e.g., no
  document.write(), innerHTML, insertAdjacentHTML(), etc.

-* If we expose some aspect of a mojo service (e.g. touch events) we
-  should expose/wrap all of it (e.g. mousewheel) so that there's no
-  cognitive cliff when interacting with that service
+ - String manipulation to generate data or code that will subsequently
+   be interpreted or parsed is a bad practice as it leads to code
+   injection vulnerabilities.

-* APIs should always spell acronyms like words (findId, not findID;
-  XmlHttpRequest, not XMLHttpRequest)
+* If we expose some aspect of a mojo service, we should expose/wrap
+  all of it, so that there's no cognitive cliff when interacting with
+  that service (where you are fine using the exposed API up to a
+  point, but beyond that have to learn all about the underlying
+  service).

-* If we extend a method to have new arguments, they must be optional
-  if there's any content using the existing method.
+Bugs
+----
+
+"Don't lick the cookie": Only assign a bug to yourself when you are
+actively working on it. If you're not working on it, leave it
+unassigned. Don't assign bugs to people unless you know they are going
+to work on it.
+
+File bugs for anything that you come across that needs doing. When you
+implement something but know it's not complete, file bugs for what you
+haven't done. That way, we can keep track of what still needs doing.
+
+Regressions
+-----------
+
+If a check-in has caused a regression on the trunk, roll back the
+check-in (even if it isn't yours) unless doing so would take longer
+than fixing the bug. When the trunk is broken, it slows down everyone
+else on the project.
+
+There is no shame in making mistakes.
+
+Questions
+---------
+
+It's always ok to ask questions. Our systems are large, nobody will be
+an expert in all the systems.
@@ -1,5 +1,5 @@
-Sky Style Guide
-===============
+Flutter Style Guide
+===================

 In general, follow our [Design Principles](design.md) for all code.

@@ -8,6 +8,22 @@ that everyone, whether reading the code for the first time or
 maintaining it for years, can quickly determine what the code does. A
 secondary goal is avoiding arguments when there are disagreements.

+All languages
+-------------
+
+Avoid checking in commented-out code. It will bitrot too fast to be
+useful, and will confuse people maintaining the code.
+
+Avoid checking in comments that ask questions (like "What should this
+be?"). Find the answers to the questions, or describe the confusion,
+including references to where you found answers. ("According to this
+specification, this should be 2.0, but according to that
+specification, it should be 3.0. We split the difference and went with
+2.5, because we didn't know what else to do.")
+
+
+
+
 Dart
 ----

@@ -15,10 +31,12 @@ In general, follow the [Dart style
 guide](https://www.dartlang.org/articles/style-guide/) for Dart code,
 except where that would contradict this page.

-Always use the Dart Analyzer. Do not check in code that increases the
-output of the analyzer unless you've filed a bug with the Dart team.
+Always use the Dart Analyzer. Avoid checking in code that increases
+the output of the analyzer unless you've filed a bug with the Dart
+team. (Use "skyanalyzer" to run the analyzer on Flutter code.)

-Use assert()s liberally.
+Use assert()s liberally to describe the contracts that you expect your
+code to follow.


 Types (i.e. classes, typedefs (function signature definitions) and
@@ -27,7 +45,21 @@ variables, constants, enum values, etc) is lowerCamelCase. Constant
 doubles and strings are prefixed with k. Prefer using a local const
 or a static const in a relevant class than using a global constant.

-Don't name your libraries (no ```library``` keyword), unless it's a
+When naming callbacks, use `FooCallback` for the typedef, `onFoo` (or,
+if there's only one and the whole purpose of the class is this
+callback, `callback`) for the callback argument or property, and
+`handleFoo` for the method that is called.
+
+If you have a callback with arguments but you want to ignore the
+arguments, name them `_`, `__`, `___`, etc. If you name any of them,
+name all of them. Always be explicit with the types of variables in
+callbacks unless you are ignoring them (and have named them with
+underscores).
+
+If you have variables or methods that are only used in release mode,
+prefix their names with `debug` or `_debug`.
+
+Avoid naming your libraries (no ```library``` keyword), unless it's a
 documented top-level library, like `painting.dart`. Name the files in
 ```lower_under_score.dart``` format.

@@ -76,20 +108,25 @@ any code that operates on all of them should operate on them in the
 same order (unless the order matters).


-All variables and arguments are typed; don't use "var", "dynamic", or
-"Object" in any case where you could figure out the actual type.
-Always specialise generic types where possible.
+All variables and arguments are typed; avoid "dynamic" or "Object" in
+any case where you could figure out the actual type. Always specialise
+generic types where possible. Explicitly type all array and map
+literals.
+
+Always avoid "var". Use "dynamic" if you are being explicit that the
+type is unknown. Use "Object" if you are being explicit that you want
+an object that implements == and hashCode.
+
+Avoid using "as". If you know the type is correct, use an assertion or
+assign to a more narrowly-typed variable (this avoids the type check
+in release mode; "as" is not compiled out in release mode). If you
+don't know whether the type is correct, check using "is" (this avoids
+the exception that "as" raises).
+

 Aim for a line length of 80 characters, but go over if breaking the
 line would make it less readable.

-Only use => when the result fits on a single line.
-
-When using ```{ }``` braces, put a space or a newline after the open
-brace and before the closing brace. (If the block is empty, the same
-space will suffice for both.) Use spaces if the whole block fits on
-one line, and newlines if you need to break it over multiple lines.
-
 When breaking an argument list into multiple lines, indent the
 arguments two characters from the previous line.

@@ -113,14 +150,22 @@ strings.
 >   print('Hello ${name.split(" ")[0]}');
 > ```

+Only use => when the result fits on a single line.
+
+When using ```{ }``` braces, put a space or a newline after the open
+brace and before the closing brace. (If the block is empty, the same
+space will suffice for both.) Use spaces if the whole block fits on
+one line, and newlines if you need to break it over multiple lines.
+
 Don't put the statement part of an "if" statement on the same line as
 the expression, even if it is short. (Doing so makes it unobvious that
 there is relevant code there. This is especially important for early
 returns.)

 If a flow control structure's statement is one line long, then don't
-use braces around it. (Keeping the code free of boilerplate or
-redundant punctuation keeps it concise and readable.)
+use braces around it, unless it's part of an "if" chain and any of the
+other blocks have more than one line. (Keeping the code free of
+boilerplate or redundant punctuation keeps it concise and readable.)

 > For example,
 > ```dart
@@ -138,6 +183,30 @@ redundant punctuation keeps it concise and readable.)
 >   }
 > ```

+> For example:
+> ```dart
+>   if (a != null)
+>     a()
+>   else if (b != null)
+>     b()
+>   else
+>     c()
+> ```
+> ...but:
+> ```dart
+>   if (a != null) {
+>     a()
+>   } else if (b != null) {
+>     b()
+>   } else {
+>     c()
+>     d()
+>   }
+> ```
+
+Use a switch if you are examining an enum (and avoid using "if" chains
+with enums), since the analyzer will warn you if you missed any of the
+values when you use a switch.

 Use the most relevant constructor or method, when there are multiple
 options.
@@ -156,11 +225,6 @@ Use for-in loops rather than forEach() where possible, since that
 saves a stack frame per iteration.


-When naming callbacks, use `FooCallback` for the typedef, `onFoo` (or,
-if there's only one and the whole purpose of the class is this
-callback, `callback`) for the callback argument or property, and
-`handleFoo` for the method that is called.
-
 When defining mutable properties that mark a class dirty when set, use
 the following pattern:

@@ -184,10 +248,6 @@ this pattern. If for some reason you don't want to use 'value', use
 Start the method with any asserts you need to validate the value.


-If you have variables or methods that are only used in release mode,
-prefix their names with `debug` or `_debug`.
-
-
 C++
 ---

@@ -197,3 +257,6 @@ Put spaces around operators in expressions.
 Java
 ----

+
+Objective C
+-----------