Skip to content

Commit

Permalink
Refactor options processor v2 (#4492)
Browse files Browse the repository at this point in the history
## Goal of this PR
The goal of this PR is to make `OptionsProcessor.ts` great again 🇺🇸 . To me it was total mess including tests. 

## What was wrong or funky?
1. the tests tested if React Native's `processColor` color works as it should even though react native has [multiple tests for this](https://github.com/facebook/react-native/blob/master/Libraries/StyleSheet/__tests__/processColor-test.js). Aka we don't have to test React Native's functions :D
2. `LayoutTreeCrawler.test.ts` was testing `OptionsProcessor`s functionality so it was removed now
3. Lots of small things since this was done before putting `noImplicitAny` to true so it wasn't made TypeScript in mind

## Result
Now `OptionsProcessor.ts` should be
1. code way more clear
2. tests are testing only things that they should
3. tests are written in clear way
4. it is easy to see from the tests what the whole function does
5. type safe for TypeScript `noImplicitAny` (when we actually turn it on)
  • Loading branch information
henrikra authored and guyca committed Dec 24, 2018
1 parent dc739de commit ee04610
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 277 deletions.
15 changes: 13 additions & 2 deletions lib/src/Navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import { TouchablePreview } from './adapters/TouchablePreview';
import { LayoutRoot, Layout } from './interfaces/Layout';
import { Options } from './interfaces/Options';
import { ComponentWrapper } from './components/ComponentWrapper';
import { OptionsProcessor } from './commands/OptionsProcessor';
import { ColorService } from './adapters/ColorService';
import { AssetService } from './adapters/AssetResolver';

export class NavigationRoot {
public readonly Element: React.ComponentType<{ elementId: any; resizeMode?: any; }>;
Expand Down Expand Up @@ -44,10 +47,18 @@ export class NavigationRoot {
this.componentEventsObserver = new ComponentEventsObserver(this.nativeEventsReceiver);
this.componentRegistry = new ComponentRegistry(this.store, this.componentEventsObserver);
this.layoutTreeParser = new LayoutTreeParser();
this.layoutTreeCrawler = new LayoutTreeCrawler(this.uniqueIdProvider, this.store);
const optionsProcessor = new OptionsProcessor(this.store, this.uniqueIdProvider, new ColorService(), new AssetService());
this.layoutTreeCrawler = new LayoutTreeCrawler(this.uniqueIdProvider, this.store, optionsProcessor);
this.nativeCommandsSender = new NativeCommandsSender();
this.commandsObserver = new CommandsObserver();
this.commands = new Commands(this.nativeCommandsSender, this.layoutTreeParser, this.layoutTreeCrawler, this.commandsObserver, this.uniqueIdProvider);
this.commands = new Commands(
this.nativeCommandsSender,
this.layoutTreeParser,
this.layoutTreeCrawler,
this.commandsObserver,
this.uniqueIdProvider,
optionsProcessor
);
this.eventsRegistry = new EventsRegistry(this.nativeEventsReceiver, this.commandsObserver, this.componentEventsObserver);

this.componentEventsObserver.registerOnceForAllComponentEvents();
Expand Down
8 changes: 8 additions & 0 deletions lib/src/adapters/AssetResolver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import * as resolveAssetSource from 'react-native/Libraries/Image/resolveAssetSource';
import { ImageRequireSource } from 'react-native';

export class AssetService {
resolveFromRequire(value: ImageRequireSource) {
return resolveAssetSource(value);
}
}
7 changes: 7 additions & 0 deletions lib/src/adapters/ColorService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { processColor } from 'react-native';

export class ColorService {
toNativeColor(inputColor: string) {
return processColor(inputColor);
}
}
20 changes: 17 additions & 3 deletions lib/src/commands/Commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { UniqueIdProvider } from '../adapters/UniqueIdProvider.mock';
import { Commands } from './Commands';
import { CommandsObserver } from '../events/CommandsObserver';
import { NativeCommandsSender } from '../adapters/NativeCommandsSender';
import { OptionsProcessor } from './OptionsProcessor';

describe('Commands', () => {
let uut: Commands;
Expand All @@ -22,12 +23,16 @@ describe('Commands', () => {
mockedNativeCommandsSender = mock(NativeCommandsSender);
nativeCommandsSender = instance(mockedNativeCommandsSender);

const mockedOptionsProcessor = mock(OptionsProcessor);
const optionsProcessor = instance(mockedOptionsProcessor);

uut = new Commands(
nativeCommandsSender,
new LayoutTreeParser(),
new LayoutTreeCrawler(new UniqueIdProvider(), store),
new LayoutTreeCrawler(new UniqueIdProvider(), store, optionsProcessor),
commandsObserver,
new UniqueIdProvider()
new UniqueIdProvider(),
optionsProcessor
);
});

Expand Down Expand Up @@ -353,7 +358,16 @@ describe('Commands', () => {
const mockParser = { parse: () => 'parsed' };
const mockCrawler = { crawl: (x: any) => x, processOptions: (x: any) => x };
commandsObserver.register(cb);
uut = new Commands(mockedNativeCommandsSender, mockParser as any, mockCrawler as any, commandsObserver, new UniqueIdProvider());
const mockedOptionsProcessor = mock(OptionsProcessor);
const optionsProcessor = instance(mockedOptionsProcessor);
uut = new Commands(
mockedNativeCommandsSender,
mockParser as any,
mockCrawler as any,
commandsObserver,
new UniqueIdProvider(),
optionsProcessor
);
});

function getAllMethodsOfUut() {
Expand Down
10 changes: 6 additions & 4 deletions lib/src/commands/Commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@ import { Options } from '../interfaces/Options';
import { Layout, LayoutRoot } from '../interfaces/Layout';
import { LayoutTreeParser } from './LayoutTreeParser';
import { LayoutTreeCrawler } from './LayoutTreeCrawler';
import { OptionsProcessor } from './OptionsProcessor';

export class Commands {
constructor(
private readonly nativeCommandsSender: NativeCommandsSender,
private readonly layoutTreeParser: LayoutTreeParser,
private readonly layoutTreeCrawler: LayoutTreeCrawler,
private readonly commandsObserver: CommandsObserver,
private readonly uniqueIdProvider: UniqueIdProvider) {
}
private readonly uniqueIdProvider: UniqueIdProvider,
private readonly optionsProcessor: OptionsProcessor
) {}

public setRoot(simpleApi: LayoutRoot) {
const input = _.cloneDeep(simpleApi);
Expand All @@ -41,15 +43,15 @@ export class Commands {

public setDefaultOptions(options: Options) {
const input = _.cloneDeep(options);
this.layoutTreeCrawler.processOptions(input);
this.optionsProcessor.processOptions(input);

this.nativeCommandsSender.setDefaultOptions(input);
this.commandsObserver.notify('setDefaultOptions', { options });
}

public mergeOptions(componentId: string, options: Options) {
const input = _.cloneDeep(options);
this.layoutTreeCrawler.processOptions(input);
this.optionsProcessor.processOptions(input);

this.nativeCommandsSender.mergeOptions(componentId, input);
this.commandsObserver.notify('mergeOptions', { componentId, options });
Expand Down
88 changes: 5 additions & 83 deletions lib/src/commands/LayoutTreeCrawler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@ import { LayoutType } from './LayoutType';
import { LayoutTreeCrawler, LayoutNode } from './LayoutTreeCrawler';
import { UniqueIdProvider } from '../adapters/UniqueIdProvider.mock';
import { Store } from '../components/Store';
import { mock, instance } from 'ts-mockito';
import { OptionsProcessor } from './OptionsProcessor';

describe('LayoutTreeCrawler', () => {
let uut: LayoutTreeCrawler;
let store: Store;

beforeEach(() => {
store = new Store();
uut = new LayoutTreeCrawler(new UniqueIdProvider(), store);
const mockedOptionsProcessor = mock(OptionsProcessor);
const optionsProcessor = instance(mockedOptionsProcessor);
uut = new LayoutTreeCrawler(new UniqueIdProvider(), store, optionsProcessor);
});

it('crawls a layout tree and adds unique id to each node', () => {
Expand Down Expand Up @@ -202,88 +206,6 @@ describe('LayoutTreeCrawler', () => {
expect(node.data.passProps).toBeUndefined();
});

describe('navigation options', () => {
let options: Record<string, any>;
let node: LayoutNode;

beforeEach(() => {
options = {};
node = { type: LayoutType.Component, data: { name: 'theComponentName', options }, children: [] };
});

it('processes colors into numeric AARRGGBB', () => {
options.someKeyColor = 'red';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xffff0000);
});

it('processes colors into numeric AARRGGBB', () => {
options.someKeyColor = 'yellow';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xffffff00);
});

it('processes numeric colors', () => {
options.someKeyColor = '#123456';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xff123456);
});

it('processes numeric colors with rrggbbAA', () => {
options.someKeyColor = 0x123456ff; // wut
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xff123456);
});

it('process colors with rgb functions', () => {
options.someKeyColor = 'rgb(255, 0, 255)';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xffff00ff);
});

it('process colors with special words', () => {
options.someKeyColor = 'fuchsia';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xffff00ff);
});

it('process colors with hsla functions', () => {
options.someKeyColor = 'hsla(360, 100%, 100%, 1.0)';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(0xffffffff);
});

it('unknown colors return undefined', () => {
options.someKeyColor = 'wut';
uut.crawl(node);
expect(node.data.options.someKeyColor).toEqual(undefined);
});

it('any keys ending with Color', () => {
options.otherKeyColor = 'red';
options.yetAnotherColor = 'blue';
options.andAnotherColor = 'rgb(0, 255, 0)';
uut.crawl(node);
expect(node.data.options.otherKeyColor).toEqual(0xffff0000);
expect(node.data.options.yetAnotherColor).toEqual(0xff0000ff);
expect(node.data.options.andAnotherColor).toEqual(0xff00ff00);
});

it('keys ending with Color case sensitive', () => {
options.otherKey_color = 'red'; // eslint-disable-line camelcase
uut.crawl(node);
expect(node.data.options.otherKey_color).toEqual('red');
});

it('any nested recursive keys ending with Color', () => {
options.innerObj = { theKeyColor: 'red' };
options.innerObj.innerMostObj = { anotherColor: 'yellow' };
uut.crawl(node);
expect(node.data.options.innerObj.theKeyColor).toEqual(0xffff0000);
expect(node.data.options.innerObj.innerMostObj.anotherColor).toEqual(0xffffff00);
});
});

describe('LayoutNode', () => {
it('convertable from same data structure', () => {
const x = {
Expand Down
18 changes: 7 additions & 11 deletions lib/src/commands/LayoutTreeCrawler.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as _ from 'lodash';
import { OptionsProcessor } from './OptionsProcessor';
import { LayoutType } from './LayoutType';
import { OptionsProcessor } from './OptionsProcessor';
import { UniqueIdProvider } from '../adapters/UniqueIdProvider';

export interface Data {
name?: string;
Expand All @@ -15,28 +16,23 @@ export interface LayoutNode {
}

export class LayoutTreeCrawler {
private optionsProcessor: OptionsProcessor;
constructor(
private readonly uniqueIdProvider: any,
public readonly store: any) {
private readonly uniqueIdProvider: UniqueIdProvider,
public readonly store: any,
private readonly optionsProcessor: OptionsProcessor
) {
this.crawl = this.crawl.bind(this);
this.processOptions = this.processOptions.bind(this);
this.optionsProcessor = new OptionsProcessor(store, uniqueIdProvider);
}

crawl(node: LayoutNode): void {
node.id = node.id || this.uniqueIdProvider.generate(node.type);
if (node.type === LayoutType.Component) {
this._handleComponent(node);
}
this.processOptions(node.data.options);
this.optionsProcessor.processOptions(node.data.options);
_.forEach(node.children, this.crawl);
}

processOptions(options) {
this.optionsProcessor.processOptions(options);
}

_handleComponent(node) {
this._assertComponentDataName(node);
this._savePropsToStore(node);
Expand Down
Loading

0 comments on commit ee04610

Please sign in to comment.