Skip to content

Commit

Permalink
Improve some doc comments
Browse files Browse the repository at this point in the history
+ That were discovered while doing the memory leak audit
  • Loading branch information
aaronlademann-wf committed Jul 18, 2017
1 parent bc46e9c commit b866700
Show file tree
Hide file tree
Showing 16 changed files with 386 additions and 170 deletions.
3 changes: 1 addition & 2 deletions lib/src/component/resize_sensor.dart
Expand Up @@ -21,7 +21,6 @@ import 'dart:html';

import 'package:meta/meta.dart';
import 'package:platform_detect/platform_detect.dart';
import 'package:react/react.dart' as react;
import 'package:over_react/over_react.dart';

/// A wrapper component that detects when its parent is resized.
Expand Down Expand Up @@ -189,7 +188,7 @@ class ResizeSensorComponent extends UiComponent<ResizeSensorProps> with _SafeAni

/// When the expand or collapse sensors are resized, builds a [ResizeSensorEvent] and calls
/// props.onResize with it. Then, calls through to [_reset()].
void _handleSensorScroll(react.SyntheticEvent _) {
void _handleSensorScroll(SyntheticEvent _) {
if (_scrollEventsToIgnore > 0) {
_scrollEventsToIgnore--;
return;
Expand Down
175 changes: 133 additions & 42 deletions lib/src/component_declaration/component_base.dart

Large diffs are not rendered by default.

52 changes: 44 additions & 8 deletions lib/src/component_declaration/component_type_checking.dart
Expand Up @@ -16,9 +16,11 @@
library over_react.component_declaration.component_type_checking;

import 'package:over_react/src/component_declaration/component_base.dart' show UiFactory;
import 'package:over_react/src/component_declaration/annotations.dart' as annotations show Component;
import 'package:over_react/src/util/react_wrappers.dart';
import 'package:react/react_client.dart';
import 'package:react/react_client/js_interop_helpers.dart';
import 'package:react/react_client/react_interop.dart';

// ----------------------------------------------------------------------
// Component type registration and internal type metadata management
Expand Down Expand Up @@ -73,10 +75,40 @@ class ComponentTypeMeta {
///
/// Used to enable inheritance in component type-checking in [isComponentOfType].
///
/// E.g., if component `Bar` is a subtype of component `Foo`, then:
/// E.g., if component `Bar` is a subtype of component `Foo`:
///
/// //
/// // foo.dart
/// //
///
/// @Factory()
/// UiFactory<FooProps> Foo;
///
/// @Component()
/// class FooComponent extends UiComponent<FooProps> {
/// // ...
/// }
///
/// //
/// // bar.dart
/// //
///
/// @Factory()
/// UiFactory<FooProps> Foo;
///
/// @Component(subtypeOf: FooComponent)
/// class BarComponent extends UiComponent<BarProps> {
/// // ...
/// }
///
/// //
/// // app.dart
/// //
///
/// isComponentOfType(Bar()(), Bar); // true (due to normal type-checking)
/// isComponentOfType(Bar()(), Foo); // true (due to parent type-checking)
///
/// > See: `subtypeOf` (within [annotations.Component])
final ReactDartComponentFactoryProxy parentType;

ComponentTypeMeta(this.isWrapper, this.parentType);
Expand Down Expand Up @@ -108,7 +140,7 @@ class ComponentTypeMeta {
///
/// Consumers of this function should be sure to take the latter case into consideration.
///
/// __CAVEAT:__ Due to type-checking limitations on JS-interop types, when [typeAlias] is a [Function],
/// > __CAVEAT:__ Due to type-checking limitations on JS-interop types, when [typeAlias] is a [Function],
/// and it is not found to be an alias for another type, it will be returned as if it were a valid type.
dynamic getComponentTypeFromAlias(dynamic typeAlias) {
/// If `typeAlias` is a factory, return its type.
Expand Down Expand Up @@ -143,19 +175,19 @@ dynamic getComponentTypeFromAlias(dynamic typeAlias) {
/// * [String] tag name (DOM components)
/// * [Function] ([ReactClass]) factory (Dart/JS composite components)
///
/// Note: It's impossible to determine know whether something is a ReactClass due to type-checking restrictions
/// for JS-interop classes, so a Function type-check is the best we can do.
/// > __NOTE:__ It's impossible to determine know whether something is a [ReactClass] due to type-checking restrictions
/// for JS-interop classes, so a Function type-check is the best we can do.
bool isPotentiallyValidComponentType(dynamic type) {
return type is Function || type is String;
}

/// Returns an [Iterable] of all component types that are ancestors of [typeAlias].
/// Returns an [Iterable] of all component types that are ancestors of [type].
///
/// For example, given components A, B, and C, where B subtypes A and C subtypes B:
///
/// getParentTypes(getTypeFromAlias(A)); // []
/// getParentTypes(getTypeFromAlias(B)); // [A].map(getTypeFromAlias)
/// getParentTypes(getTypeFromAlias(C)); // [B, A].map(getTypeFromAlias)
/// getParentTypes(getComponentTypeFromAlias(A)); // []
/// getParentTypes(getComponentTypeFromAlias(B)); // [A].map(getTypeFromAlias)
/// getParentTypes(getComponentTypeFromAlias(C)); // [B, A].map(getTypeFromAlias)
Iterable<dynamic> getParentTypes(dynamic type) sync* {
assert(isPotentiallyValidComponentType(type) &&
'`type` should be a valid component type (and not null or a type alias).' is String);
Expand All @@ -182,6 +214,8 @@ Iterable<dynamic> getParentTypes(dynamic type) sync* {
/// * [ReactComponentFactoryProxy]
/// * [ReactClass] component factory
/// * [String] tag name (DOM components only)
///
/// > Related: [isValidElementOfType]
bool isComponentOfType(ReactElement instance, dynamic typeAlias, {
bool traverseWrappers: true,
bool matchParentTypes: true
Expand Down Expand Up @@ -228,6 +262,8 @@ bool isComponentOfType(ReactElement instance, dynamic typeAlias, {
/// * [ReactComponentFactoryProxy]
/// * [ReactClass] component factory
/// * [String] tag name (DOM components only)
///
/// > Related: [isComponentOfType]
bool isValidElementOfType(dynamic instance, dynamic typeAlias) {
return isValidElement(instance) && isComponentOfType(instance, typeAlias);
}
8 changes: 6 additions & 2 deletions lib/src/component_declaration/flux_component.dart
Expand Up @@ -66,6 +66,8 @@ abstract class FluxUiProps<ActionsT, StoresT> extends UiProps {
/// the resulting component.
///
/// Use with the over_react transformer via the `@Component()` ([annotations.Component]) annotation.
///
/// > Related: [FluxUiStatefulComponent]
abstract class FluxUiComponent<TProps extends FluxUiProps> extends UiComponent<TProps>
with _FluxComponentMixin<TProps>, BatchedRedraws {
// Redeclare these lifecycle methods with `mustCallSuper`, since `mustCallSuper` added to methods within
Expand Down Expand Up @@ -102,6 +104,8 @@ abstract class FluxUiComponent<TProps extends FluxUiProps> extends UiComponent<T
/// the resulting component.
///
/// Use with the over_react transformer via the `@Component()` ([annotations.Component]) annotation.
///
/// > Related: [FluxUiComponent]
abstract class FluxUiStatefulComponent<TProps extends FluxUiProps, TState extends UiState>
extends UiStatefulComponent<TProps, TState>
with _FluxComponentMixin<TProps>, BatchedRedraws {
Expand Down Expand Up @@ -156,9 +160,9 @@ abstract class _FluxComponentMixin<TProps extends FluxUiProps> implements Batche

handlers.forEach((store, handler) {
String message = 'Cannot listen to a disposed/disposing Store.';

var isDisposedOrDisposing = store.isDisposedOrDisposing ?? false;

assert(!isDisposedOrDisposing, '$message This can be caused by BatchedRedraws '
'mounting the component asynchronously after the store has been disposed. If you are '
'in a test environment, try adding an `await window.animationFrame;` before disposing your '
Expand Down
134 changes: 98 additions & 36 deletions lib/src/util/class_names.dart
Expand Up @@ -17,14 +17,15 @@ library over_react.class_names;

import 'dart:collection';

// Must import these consts because they are used in the transformed code.
// ignore: unused_import
import 'package:over_react/over_react.dart' show PropDescriptor, ConsumedProps;
import 'package:over_react/over_react.dart' show
// Must import these consts because they are used in the transformed code.
PropDescriptor, ConsumedProps, // ignore: unused_shown_name
UiComponent, UiProps;
import 'package:over_react/src/component_declaration/annotations.dart';

/// Typed getters/setters for props related to CSS class manipulation, and used by all over_react components.
/// Typed getters/setters for props related to CSS class manipulation.
///
/// To be used as a mixin for React components and builders.
/// Universally available on all OverReact components via [UiProps].
@PropsMixin(keyNamespace: '')
abstract class CssClassPropsMixin {
Map get props;
Expand All @@ -42,39 +43,44 @@ abstract class CssClassPropsMixin {
String classNameBlacklist;
}

/// A MapView with the typed getters/setters for all CSS-class-related props.
/// A `MapView` with typed getters/setters for all CSS-class-related props.
class CssClassPropsMapView extends MapView with CssClassPropsMixin {
/// Create a new instance backed by the specified map.
CssClassPropsMapView(Map map) : super(map);

/// The props to be manipulated via the getters/setters.
/// In this case, it's the current MapView object.
///
/// In this case, it's the current [MapView] object.
@override
Map get props => this;
}

/// StringBuffer-backed className builder optimized for adding classNames, with support for blacklisting CSS classes.
/// A [StringBuffer]-backed CSS class builder optimized for adding classNames,
/// with support for blacklisting CSS classes.
class ClassNameBuilder {
StringBuffer _classNamesBuffer = new StringBuffer();
StringBuffer _blacklistBuffer;

/// Creates a new, empty ClassNameBuilder.
/// Creates a new, empty `ClassNameBuilder` instance.
///
/// > Related: [UiComponent.forwardingClassNameBuilder]
ClassNameBuilder();

/// Creates a new ClassNameBuilder with className and blacklist values added from [CssClassProps.className] and
/// [CssClassProps.classNameBlackList], if they are specified.
/// Creates a new `ClassNameBuilder` with the [CssClassPropsMixin.className] values and
/// excludes the [CssClassPropsMixin.classNameBlacklist] values if specified within the
/// provided [props] Map.
///
/// This method gracefully handles null [props], as well as unspecified/null prop values.
ClassNameBuilder.fromProps(Map props) {
addFromProps(props);
}

/// Adds the className and blacklist values from a [props] Map, using the
/// [CssClassProps.className] and [CssClassProps.classNameBlackList] values.
/// Adds the [CssClassPropsMixin.className] and excludes the [CssClassPropsMixin.classNameBlacklist] values
/// if specified within the provided [props] Map.
///
/// This method gracefully handles null [props], as well as unspecified/null prop values.
///
/// This method, along with [toProps], is useful for merging sets of className/blacklist props.
/// > This method, along with [toProps], are useful for merging sets of className/blacklist props.
void addFromProps(Map props) {
if (props == null) {
return;
Expand All @@ -87,12 +93,31 @@ class ClassNameBuilder {
..blacklist(cssClassProps.classNameBlacklist);
}

/// Adds a className string. May be a single CSS class 'token', or multiple space-delimited classes,
/// IF [should] is true, otherwise, does nothing (convenience for helping to inline addition conditionals).
/// Adds all of the CSS classes represented by a space-delimited list of [className]s
/// if [shouldAdd] is `true`.
///
/// Does not check for / remove duplicate CSS classes.
///
/// Is a noop if [shouldAdd] is `false`, [className] is `null` or [className] is an empty string.
///
/// __[shouldAdd] makes conditional CSS classes a breeze:__
///
/// There is no checking for duplicate CSS classes.
void add(String className, [bool should = true]) {
if (!should || className == null || className == '') {
/// @override
/// render() {
/// var classes = forwardingClassNameBuilder()
/// ..add('foo')
/// ..add('foo--is-active', state.isActive)
/// ..add('foo--is-disabled', state.isDisabled);
///
/// return (Dom.div()
/// ..addProps(copyUnconsumedDomProps())
/// ..className = classes.toClassName()
/// )(props.children);
/// }
///
/// > Related: [blacklist]
void add(String className, [bool shouldAdd = true]) {
if (!shouldAdd || className == null || className == '') {
return;
}

Expand All @@ -102,12 +127,16 @@ class ClassNameBuilder {
_classNamesBuffer.write(className);
}

/// Adds all of the CSS classes represented by [className] (a space-delimited list) to the blacklist,
/// IF [should] is true, otherwise, does nothing (convenience for helping to inline blacklisting conditionals).
/// Blacklists all of the CSS classes represented by a space-delimited list of [className]s
/// if [shouldBlacklist] is `true`.
///
/// Classes added to the blacklist will not appear in the result of [toClassName].
void blacklist(String className, [bool should = true]) {
if (!should || className == null || className == '') {
///
/// Is a noop if [shouldBlacklist] is `false`, [className] is `null` or [className] is an empty string.
///
/// > Related: [add]
void blacklist(String className, [bool shouldBlacklist = true]) {
if (!shouldBlacklist || className == null || className == '') {
return;
}

Expand All @@ -121,37 +150,70 @@ class ClassNameBuilder {
_blacklistBuffer.write(className);
}

/// Returns a String representation of the built className, which includes any added classes, and none of the blacklisted classes.
/// Returns a String representation of the built className, which includes any added classes,
/// and none of the blacklisted classes.
///
/// Duplicate classes will be added.
/// Does not check for / remove duplicate CSS classes.
///
/// __Necessary to convert a `ClassNameBuilder` instance into a valid [CssClassPropsMixin.className] value:__
///
/// @override
/// render() {
/// var classes = forwardingClassNameBuilder()
/// ..add('foo')
/// ..add('foo--is-active', state.isActive)
/// ..add('foo--is-disabled', state.isDisabled);
///
/// return (Dom.div()
/// ..addProps(copyUnconsumedDomProps())
/// ..className = classes.toClassName()
/// )(props.children);
/// }
///
/// > Related: [toClassNameBlacklist]
String toClassName() {
String className = _classNamesBuffer.toString();

if (_blacklistBuffer != null && _blacklistBuffer.isNotEmpty) {
List blacklistedClasses = splitSpaceDelimitedString(_blacklistBuffer.toString());

className = splitSpaceDelimitedString(className)
.where((String cssClass) => !blacklistedClasses.contains(cssClass))
.join(' ');
.where((String cssClass) => !blacklistedClasses.contains(cssClass))
.join(' ');
}

return className;
}

/// Returns a String representation of only the blacklisted classes.
/// Useful for blacklist forwarding.
///
/// Duplicate classes will be added.
/// Does not check for / remove duplicate CSS classes.
///
/// __Useful for blacklist forwarding:__
///
/// @override
/// render() {
/// var classes = forwardingClassNameBuilder()
/// ..blacklist('some-class-found-in-the-forwarding-builder', state.becauseReasons);
///
/// return (NestedComponent()
/// ..addProps(copyUnconsumedProps())
/// ..className = classes.toClassName()
/// ..classNameBlacklist = classes.toClassNameBlacklist()
/// )(props.children);
/// }
///
/// > Related: [toClassName]
String toClassNameBlacklist() {
return _blacklistBuffer == null || _blacklistBuffer.isEmpty
? null
: _blacklistBuffer.toString();
? null
: _blacklistBuffer.toString();
}

/// Returns a Map with the [CssClassProps.className] and [CssClassProps.classNameBlackList] props
/// Returns a Map with the [CssClassPropsMixin.className] and [CssClassPropsMixin.classNameBlacklist] props
/// populated from the return values of [toClassName] and [toClassNameBlacklist], respectively.
///
/// This method, along with [addFromProps], is useful for merging sets of className/blacklist props.
/// > This method, along with [addFromProps], are useful for merging sets of className/blacklist props.
Map toProps() {
return new CssClassPropsMapView({})
..className = toClassName()
Expand All @@ -164,13 +226,13 @@ class ClassNameBuilder {
}
}

/// Returns a List of space-delimited tokens efficiently split from the specified string.
/// Returns a List of space-delimited tokens efficiently split from the specified [string].
///
/// Useful for splitting CSS class name strings into class tokens, or `data-test-id` values into individual test IDs.
/// Useful for splitting CSS className strings into class tokens, or `data-test-id` values into individual test IDs.
///
/// Handles leading and trailing spaces, as well as token separated by multiple spaces.
///
/// Example:
/// __Example:__
///
/// splitSpaceDelimitedString(' foo bar baz') // ['foo', 'bar', 'baz']
List<String> splitSpaceDelimitedString(String string) {
Expand Down

0 comments on commit b866700

Please sign in to comment.