Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

refactor(*): separate jqLite/compile/sce camelCasing logic #15249

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
"BOOLEAN_ATTR": false,
"ALIASED_ATTR": false,
"jqNextId": false,
"camelCase": false,
"fnCamelCaseReplace": false,
"jqLitePatchJQueryRemove": false,
"JQLite": false,
"jqLiteClone": false,
Expand Down
31 changes: 20 additions & 11 deletions src/jqLite.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,22 +136,31 @@ JQLite._data = function(node) {
function jqNextId() { return ++jqId; }


var SPECIAL_CHARS_REGEXP = /([:\-_]+(.))/g;
var MOZ_HACK_REGEXP = /^moz([A-Z])/;
var DASH_LOWERCASE_REGEXP = /-([a-z])/g;
Copy link
Member

@gkalpak gkalpak Oct 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should match [A-Z] as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't. That's the whole point of this change!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I even wrote some tests to ensure that's not getting converted. ;)

var MS_HACK_REGEXP = /^-ms-/;
var MOUSE_EVENT_MAP = { mouseleave: 'mouseout', mouseenter: 'mouseover' };
var jqLiteMinErr = minErr('jqLite');

/**
* Converts snake_case to camelCase.
* Also there is special case for Moz prefix starting with upper case letter.
* Converts kebab-case to camelCase.
* There is also a special case for the ms prefix starting with a lowercase letter.
* @param name Name to normalize
*/
function camelCase(name) {
return name.
replace(SPECIAL_CHARS_REGEXP, function(_, separator, letter, offset) {
return offset ? letter.toUpperCase() : letter;
}).
replace(MOZ_HACK_REGEXP, 'Moz$1');
function cssKebabToCamel(name) {
return kebabToCamel(name.replace(MS_HACK_REGEXP, 'ms-'));
}

function fnCamelCaseReplace(all, letter) {
return letter.toUpperCase();
}

/**
* Converts kebab-case to camelCase.
* @param name Name to normalize
*/
function kebabToCamel(name) {
return name
.replace(DASH_LOWERCASE_REGEXP, fnCamelCaseReplace);
}

var SINGLE_TAG_REGEXP = /^<([\w-]+)\s*\/?>(?:<\/\1>|)$/;
Expand Down Expand Up @@ -633,7 +642,7 @@ forEach({
hasClass: jqLiteHasClass,

css: function(element, name, value) {
name = camelCase(name);
name = cssKebabToCamel(name);

if (isDefined(value)) {
element.style[name] = value;
Expand Down
6 changes: 5 additions & 1 deletion src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -3590,12 +3590,16 @@ SimpleChange.prototype.isFirstChange = function() { return this.previousValue ==


var PREFIX_REGEXP = /^((?:x|data)[:\-_])/i;
var SPECIAL_CHARS_REGEXP = /[:\-_]+(.)/g;

/**
* Converts all accepted directives format into proper directive name.
* @param name Name to normalize
*/
function directiveNormalize(name) {
return camelCase(name.replace(PREFIX_REGEXP, ''));
return name
.replace(PREFIX_REGEXP, '')
.replace(SPECIAL_CHARS_REGEXP, fnCamelCaseReplace);
}

/**
Expand Down
13 changes: 10 additions & 3 deletions src/ng/sce.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ var SCE_CONTEXTS = {

// Helper functions follow.

var UNDERSCORE_LOWERCASE_REGEXP = /_([a-z])/g;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too, I wonder what might break by only matching lowercase letters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing. ;) This is a purely internal function, only used to iterate once through a few constant strings during the initialization.


function snakeToCamel(name) {
return name
.replace(UNDERSCORE_LOWERCASE_REGEXP, fnCamelCaseReplace);
}

function adjustMatcher(matcher) {
if (matcher === 'self') {
return matcher;
Expand Down Expand Up @@ -1054,13 +1061,13 @@ function $SceProvider() {

forEach(SCE_CONTEXTS, function(enumValue, name) {
var lName = lowercase(name);
sce[camelCase('parse_as_' + lName)] = function(expr) {
sce[snakeToCamel('parse_as_' + lName)] = function(expr) {
return parse(enumValue, expr);
};
sce[camelCase('get_trusted_' + lName)] = function(value) {
sce[snakeToCamel('get_trusted_' + lName)] = function(value) {
return getTrusted(enumValue, value);
};
sce[camelCase('trust_as_' + lName)] = function(value) {
sce[snakeToCamel('trust_as_' + lName)] = function(value) {
return trustAs(enumValue, value);
};
});
Expand Down
3 changes: 2 additions & 1 deletion test/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@
/* jqLite.js */
"BOOLEAN_ATTR": false,
"jqNextId": false,
"camelCase": false,
"kebabToCamel": false,
"fnCamelCaseReplace": false,
"jqLitePatchJQueryRemove": false,
"JQLite": false,
"jqLiteClone": false,
Expand Down
133 changes: 121 additions & 12 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,105 @@ describe('jqLite', function() {
expect(jqA.css('z-index')).toBeOneOf('7', 7);
expect(jqA.css('zIndex')).toBeOneOf('7', 7);
});

it('should leave non-dashed strings alone', function() {
var jqA = jqLite(a);

jqA.css('foo', 'foo');
jqA.css('fooBar', 'bar');

expect(a.style.foo).toBe('foo');
expect(a.style.fooBar).toBe('bar');
});

it('should convert dash-separated strings to camelCase', function() {
var jqA = jqLite(a);

jqA.css('foo-bar', 'foo');
jqA.css('foo-bar-baz', 'bar');
jqA.css('foo:bar_baz', 'baz');

expect(a.style.fooBar).toBe('foo');
expect(a.style.fooBarBaz).toBe('bar');
expect(a.style['foo:bar_baz']).toBe('baz');
});

it('should convert leading dashes followed by a lowercase letter', function() {
var jqA = jqLite(a);

jqA.css('-foo-bar', 'foo');

expect(a.style.FooBar).toBe('foo');
});

it('should not convert slashes followed by a non-letter', function() {
// jQuery 2.x had different behavior; skip the test.
if (isJQuery2x()) return;

var jqA = jqLite(a);

jqA.css('foo-42- -a-B', 'foo');

expect(a.style['foo-42- A-B']).toBe('foo');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a crazy case. Do we really care that this is what it does?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't expect keys exactly like that to be used in practice but this string tests the fact that only -[a-z] gets converted pretty well. I can split it into separate cases if you prefer but I thought one string should suffice.

});

it('should convert the -ms- prefix to ms instead of Ms', function() {
var jqA = jqLite(a);

jqA.css('-ms-foo-bar', 'foo');
jqA.css('-moz-foo-bar', 'bar');
jqA.css('-webkit-foo-bar', 'baz');

expect(a.style.msFooBar).toBe('foo');
expect(a.style.MozFooBar).toBe('bar');
expect(a.style.WebkitFooBar).toBe('baz');
});

it('should not collapse sequences of dashes', function() {
var jqA = jqLite(a);

jqA.css('foo---bar-baz--qaz', 'foo');

expect(a.style['foo--BarBaz-Qaz']).toBe('foo');
});


it('should read vendor prefixes with the special -ms- exception', function() {
// jQuery uses getComputedStyle() in a css getter so these tests would fail there.
if (!_jqLiteMode) return;

var jqA = jqLite(a);

a.style.WebkitFooBar = 'webkit-uppercase';
a.style.webkitFooBar = 'webkit-lowercase';

a.style.MozFooBaz = 'moz-uppercase';
a.style.mozFooBaz = 'moz-lowercase';

a.style.MsFooQaz = 'ms-uppercase';
a.style.msFooQaz = 'ms-lowercase';

expect(jqA.css('-webkit-foo-bar')).toBe('webkit-uppercase');
expect(jqA.css('-moz-foo-baz')).toBe('moz-uppercase');
expect(jqA.css('-ms-foo-qaz')).toBe('ms-lowercase');
});

it('should write vendor prefixes with the special -ms- exception', function() {
var jqA = jqLite(a);

jqA.css('-webkit-foo-bar', 'webkit');
jqA.css('-moz-foo-baz', 'moz');
jqA.css('-ms-foo-qaz', 'ms');

expect(a.style.WebkitFooBar).toBe('webkit');
expect(a.style.webkitFooBar).not.toBeDefined();

expect(a.style.MozFooBaz).toBe('moz');
expect(a.style.mozFooBaz).not.toBeDefined();

expect(a.style.MsFooQaz).not.toBeDefined();
expect(a.style.msFooQaz).toBe('ms');
});
});


Expand Down Expand Up @@ -2267,25 +2366,35 @@ describe('jqLite', function() {
});


describe('camelCase', function() {
describe('kebabToCamel', function() {
it('should leave non-dashed strings alone', function() {
expect(camelCase('foo')).toBe('foo');
expect(camelCase('')).toBe('');
expect(camelCase('fooBar')).toBe('fooBar');
expect(kebabToCamel('foo')).toBe('foo');
expect(kebabToCamel('')).toBe('');
expect(kebabToCamel('fooBar')).toBe('fooBar');
});

it('should convert dash-separated strings to camelCase', function() {
expect(kebabToCamel('foo-bar')).toBe('fooBar');
expect(kebabToCamel('foo-bar-baz')).toBe('fooBarBaz');
expect(kebabToCamel('foo:bar_baz')).toBe('foo:bar_baz');
});

it('should convert leading dashes followed by a lowercase letter', function() {
expect(kebabToCamel('-foo-bar')).toBe('FooBar');
});

it('should covert dash-separated strings to camelCase', function() {
expect(camelCase('foo-bar')).toBe('fooBar');
expect(camelCase('foo-bar-baz')).toBe('fooBarBaz');
expect(camelCase('foo:bar_baz')).toBe('fooBarBaz');
it('should not convert dashes followed by a non-letter', function() {
expect(kebabToCamel('foo-42- -a-B')).toBe('foo-42- A-B');
});

it('should not convert browser specific css properties in a special way', function() {
expect(kebabToCamel('-ms-foo-bar')).toBe('MsFooBar');
expect(kebabToCamel('-moz-foo-bar')).toBe('MozFooBar');
expect(kebabToCamel('-webkit-foo-bar')).toBe('WebkitFooBar');
});

it('should covert browser specific css properties', function() {
expect(camelCase('-moz-foo-bar')).toBe('MozFooBar');
expect(camelCase('-webkit-foo-bar')).toBe('webkitFooBar');
expect(camelCase('-webkit-foo-bar')).toBe('webkitFooBar');
it('should not collapse sequences of dashes', function() {
expect(kebabToCamel('foo---bar-baz--qaz')).toBe('foo--BarBaz-Qaz');
});
});

Expand Down