Skip to content

Commit

Permalink
perf: Improve performance of toTitleCase, register with lower and Tit…
Browse files Browse the repository at this point in the history
…leCase (videojs#6148)
  • Loading branch information
brandonocasey authored and gkatsev committed Aug 7, 2019
1 parent 8610f99 commit 266cb15
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 32 deletions.
17 changes: 7 additions & 10 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as Dom from './utils/dom.js';
import DomData from './utils/dom-data';
import * as Fn from './utils/fn.js';
import * as Guid from './utils/guid.js';
import toTitleCase from './utils/to-title-case.js';
import {toTitleCase, toLowerCase} from './utils/string-cases.js';
import mergeOptions from './utils/merge-options.js';
import computedStyle from './utils/computed-style';

Expand Down Expand Up @@ -360,8 +360,6 @@ class Component {
return;
}

name = toTitleCase(name);

return this.childNameIndex_[name];
}

Expand Down Expand Up @@ -435,6 +433,7 @@ class Component {

if (componentName) {
this.childNameIndex_[componentName] = component;
this.childNameIndex_[toLowerCase(componentName)] = component;
}

// Add the UI object's element to the container div (box)
Expand Down Expand Up @@ -483,7 +482,8 @@ class Component {
component.parentComponent_ = null;

this.childIndex_[component.id()] = null;
this.childNameIndex_[component.name()] = null;
this.childNameIndex_[toTitleCase(component.name())] = null;
this.childNameIndex_[toLowerCase(component.name())] = null;

const compEl = component.el();

Expand Down Expand Up @@ -1545,6 +1545,7 @@ class Component {
}

Component.components_[name] = ComponentToRegister;
Component.components_[toLowerCase(name)] = ComponentToRegister;

return ComponentToRegister;
}
Expand All @@ -1564,15 +1565,11 @@ class Component {
* return that if it exists.
*/
static getComponent(name) {
if (!name) {
if (!name || !Component.components_) {
return;
}

name = toTitleCase(name);

if (Component.components_ && Component.components_[name]) {
return Component.components_[name];
}
return Component.components_[name];
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/js/control-bar/text-track-controls/chapters-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import TextTrackButton from './text-track-button.js';
import Component from '../../component.js';
import ChaptersTrackMenuItem from './chapters-track-menu-item.js';
import toTitleCase from '../../utils/to-title-case.js';
import {toTitleCase} from '../../utils/string-cases.js';

/**
* The button component for toggling and selecting chapters
Expand Down
2 changes: 1 addition & 1 deletion src/js/control-bar/text-track-controls/subs-caps-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import TextTrackButton from './text-track-button.js';
import Component from '../../component.js';
import CaptionSettingsMenuItem from './caption-settings-menu-item.js';
import SubsCapsMenuItem from './subs-caps-menu-item.js';
import toTitleCase from '../../utils/to-title-case.js';
import {toTitleCase} from '../../utils/string-cases.js';
/**
* The button component for toggling and selecting captions and/or subtitles
*
Expand Down
2 changes: 1 addition & 1 deletion src/js/menu/menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Button from '../button.js';
import Component from '../component.js';
import Menu from './menu.js';
import * as Dom from '../utils/dom.js';
import toTitleCase from '../utils/to-title-case.js';
import {toTitleCase} from '../utils/string-cases.js';
import { IS_IOS } from '../utils/browser.js';
import keycode from 'keycode';

Expand Down
2 changes: 1 addition & 1 deletion src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import * as Guid from './utils/guid.js';
import * as browser from './utils/browser.js';
import {IE_VERSION, IS_CHROME, IS_WINDOWS} from './utils/browser.js';
import log, { createLogger } from './utils/log.js';
import toTitleCase, { titleCaseEquals } from './utils/to-title-case.js';
import {toTitleCase, titleCaseEquals} from './utils/string-cases.js';
import { createTimeRange } from './utils/time-ranges.js';
import { bufferedPercent } from './utils/buffer.js';
import * as stylesheet from './utils/stylesheet.js';
Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import document from 'global/document';
import window from 'global/window';
import {assign} from '../utils/obj';
import mergeOptions from '../utils/merge-options.js';
import toTitleCase from '../utils/to-title-case.js';
import {toTitleCase} from '../utils/string-cases.js';
import {NORMAL as TRACK_TYPES} from '../tracks/track-types';
import setupSourceset from './setup-sourceset';

Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import Component from '../component.js';
import Tech from './tech.js';
import toTitleCase from '../utils/to-title-case.js';
import {toTitleCase} from '../utils/string-cases.js';
import mergeOptions from '../utils/merge-options.js';

/**
Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* @module middleware
*/
import { assign } from '../utils/obj.js';
import toTitleCase from '../utils/to-title-case.js';
import {toTitleCase} from '../utils/string-cases.js';

const middlewares = {};
const middlewareInstances = {};
Expand Down
7 changes: 4 additions & 3 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import window from 'global/window';
import document from 'global/document';
import {isPlain} from '../utils/obj';
import * as TRACK_TYPES from '../tracks/track-types';
import toTitleCase from '../utils/to-title-case';
import {toTitleCase, toLowerCase} from '../utils/string-cases.js';
import vtt from 'videojs-vtt.js';

/**
Expand Down Expand Up @@ -920,6 +920,7 @@ class Tech extends Component {
name = toTitleCase(name);

Tech.techs_[name] = tech;
Tech.techs_[toLowerCase(name)] = tech;
if (name !== 'Tech') {
// camel case the techName for use in techOrder
Tech.defaultTechOrder_.push(name);
Expand All @@ -941,12 +942,12 @@ class Tech extends Component {
return;
}

name = toTitleCase(name);

if (Tech.techs_ && Tech.techs_[name]) {
return Tech.techs_[name];
}

name = toTitleCase(name);

if (window && window.videojs && window.videojs[name]) {
log.warn(`The ${name} tech was added to the videojs object when it should be registered using videojs.registerTech(name, tech)`);
return window.videojs[name];
Expand Down
33 changes: 24 additions & 9 deletions src/js/utils/to-title-case.js → src/js/utils/string-cases.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
/**
* @file to-title-case.js
* @module to-title-case
* @file string-cases.js
* @module to-lower-case
*/

/**
* Lowercase the first letter of a string.
*
* @param {string} string
* String to be lowercased
*
* @return {string}
* The string with a lowercased first letter
*/
export const toLowerCase = function(string) {
if (typeof string !== 'string') {
return string;
}

return string.replace(/./, (w) => w.toLowerCase());
};

/**
* Uppercase the first letter of a string.
*
Expand All @@ -12,15 +29,13 @@
* @return {string}
* The string with an uppercased first letter
*/
function toTitleCase(string) {
export const toTitleCase = function(string) {
if (typeof string !== 'string') {
return string;
}

return string.charAt(0).toUpperCase() + string.slice(1);
}

export default toTitleCase;
return string.replace(/./, (w) => w.toUpperCase());
};

/**
* Compares the TitleCase versions of the two strings for equality.
Expand All @@ -34,6 +49,6 @@ export default toTitleCase;
* @return {boolean}
* Whether the TitleCase versions of the strings are equal
*/
export function titleCaseEquals(str1, str2) {
export const titleCaseEquals = function(str1, str2) {
return toTitleCase(str1) === toTitleCase(str2);
}
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-env qunit */
import toTitleCase, { titleCaseEquals } from '../../../src/js/utils/to-title-case.js';
import {toLowerCase, toTitleCase, titleCaseEquals} from '../../../src/js/utils/string-cases.js';

QUnit.module('to-title-case');
QUnit.module('string-cases');

QUnit.test('should make a string start with an uppercase letter', function(assert) {
QUnit.test('toTitleCase should make a string start with an uppercase letter', function(assert) {
const foo = toTitleCase('bar');

assert.ok(foo === 'Bar');
Expand All @@ -20,3 +20,9 @@ QUnit.test('titleCaseEquals compares whether the TitleCase of two strings is equ
assert.notOk(titleCaseEquals('foobar', 'fooBar'), 'foobar does not equal fooBar');
assert.notOk(titleCaseEquals('fooBar', 'FOOBAR'), 'fooBar does not equal fooBAR');
});

QUnit.test('toLowerCase should make a string start with a lowercase letter', function(assert) {
const foo = toLowerCase('BAR');

assert.ok(foo === 'bAR');
});

0 comments on commit 266cb15

Please sign in to comment.