Permalink
Comparing changes
Open a pull request
- 2 commits
- 6 files changed
- 2 commit comments
- 1 contributor
Commits on Oct 13, 2016
jqLite needs camelCase for it's css method; it should only convert one dash followed by a lowercase letter to an uppercase one; it shouldn't touch underscores, colons or collapse multiple dashes into one. This is behavior of jQuery 3 as well. Also, jqLite's css camelCasing logic was put in a separate function and refactored: now the properties starting from an uppercase letter are used by default (i.e. Webkit, not webkit) and the only exception is for the -ms- prefix that is converted to ms, not Ms. This makes the logic clearer as we're just always changing a dash followed by a lowercase letter by an uppercase one; this is also how it works in jQuery. The camelCasing for the $compile and $sce services retains the previous behaviour. Ref #15126 Fix #7744 BREAKING CHANGE: before, when Angular was used without jQuery, the key passed to the css method was more heavily camelCased; now only a single (!) hyphen followed by a lowercase letter is getting transformed. This also affects APIs that rely on the css method, like ngStyle. If you use Angular with jQuery, it already behaved in this way so no changes are needed on your part. To migrate the code follow the example below: Before: HTML: // All five versions used to be equivalent. <div ng-style={background_color: 'blue'}></div> <div ng-style={'background:color': 'blue'}></div> <div ng-style={'background-color': 'blue'}></div> <div ng-style={'background--color': 'blue'}></div> <div ng-style={backgroundColor: 'blue'}></div> JS: // All five versions used to be equivalent. elem.css('background_color', 'blue'); elem.css('background:color', 'blue'); elem.css('background-color', 'blue'); elem.css('background--color', 'blue'); elem.css('backgroundColor', 'blue'); // All five versions used to be equivalent. var bgColor = elem.css('background_color'); var bgColor = elem.css('background:color'); var bgColor = elem.css('background-color'); var bgColor = elem.css('background--color'); var bgColor = elem.css('backgroundColor'); After: HTML: // Previous five versions are no longer equivalent but these two still are. <div ng-style={'background-color': 'blue'}></div> <div ng-style={backgroundColor: 'blue'}></div> JS: // Previous five versions are no longer equivalent but these two still are. elem.css('background-color', 'blue'); elem.css('backgroundColor', 'blue'); // Previous five versions are no longer equivalent but these two still are. var bgColor = elem.css('background-color'); var bgColor = elem.css('backgroundColor');
This change aligns jqLite with jQuery 3. The relevant bit of jQuery code is https://github.com/jquery/jquery/blob/3.1.1/src/data/Data.js Close #15126 BREAKING CHANGE: Previously, keys passed to the data method were left untouched. Now they are internally camelCased similarly to how jQuery handles it, i.e. only single (!) hyphens followed by a lowercase letter get converted to an uppercase letter. This means keys `a-b` and `aB` represent the same data piece; writing to one of them will also be reflected if you ask for the other one. If you use Angular with jQuery, it already behaved in this way so no changes are required on your part. To migrate the code follow the examples below: BEFORE: /* 1 */ elem.data('my-key', 2); elem.data('myKey', 3); /* 2 */ elem.data('foo-bar', 42); elem.data()['foo-bar']; // 42 elem.data()['fooBar']; // undefined /* 3 */ elem.data()['foo-bar'] = 1; elem.data()['fooBar'] = 2; elem.data()['foo-bar']; // 1 AFTER: /* 1 */ // Rename one of the keys as they would now map to the same data slot. elem.data('my-key', 2); elem.data('my-key2', 3); /* 2 */ elem.data('foo-bar', 42); elem.data()['foo-bar']; // undefined elem.data()['fooBar']; // 42 /* 3 */ elem.data()['foo-bar'] = 1; elem.data()['fooBar'] = 2; elem.data()['foo-bar']; // 2
Unified
Split
Showing
with
198 additions
and 32 deletions.
- +1 −1 src/.eslintrc.json
- +26 −14 src/jqLite.js
- +5 −1 src/ng/compile.js
- +10 −3 src/ng/sce.js
- +2 −1 test/.eslintrc.json
- +154 −12 test/jqLiteSpec.js
| @@ -123,7 +123,7 @@ | ||
| "BOOLEAN_ATTR": false, | ||
| "ALIASED_ATTR": false, | ||
| "jqNextId": false, | ||
| "camelCase": false, | ||
| "fnCamelCaseReplace": false, | ||
| "jqLitePatchJQueryRemove": false, | ||
| "JQLite": false, | ||
| "jqLiteClone": false, | ||
| @@ -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; | ||
| 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>|)$/; | ||
| @@ -385,6 +394,7 @@ function jqLiteExpandoStore(element, createIfNecessary) { | ||
|
|
||
| function jqLiteData(element, key, value) { | ||
| if (jqLiteAcceptsData(element)) { | ||
| var prop; | ||
|
|
||
| var isSimpleSetter = isDefined(value); | ||
| var isSimpleGetter = !isSimpleSetter && key && !isObject(key); | ||
| @@ -393,16 +403,18 @@ function jqLiteData(element, key, value) { | ||
| var data = expandoStore && expandoStore.data; | ||
|
|
||
| if (isSimpleSetter) { // data('key', value) | ||
| data[key] = value; | ||
| data[kebabToCamel(key)] = value; | ||
| } else { | ||
| if (massGetter) { // data() | ||
| return data; | ||
| } else { | ||
| if (isSimpleGetter) { // data('key') | ||
| // don't force creation of expandoStore if it doesn't exist yet | ||
| return data && data[key]; | ||
| return data && data[kebabToCamel(key)]; | ||
| } else { // mass-setter: data({key1: val1, key2: val2}) | ||
| extend(data, key); | ||
| for (prop in key) { | ||
| data[kebabToCamel(prop)] = key[prop]; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| @@ -633,7 +645,7 @@ forEach({ | ||
| hasClass: jqLiteHasClass, | ||
|
|
||
| css: function(element, name, value) { | ||
| name = camelCase(name); | ||
| name = cssKebabToCamel(name); | ||
|
|
||
| if (isDefined(value)) { | ||
| element.style[name] = value; | ||
| @@ -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); | ||
| } | ||
|
|
||
| /** | ||
| @@ -27,6 +27,13 @@ var SCE_CONTEXTS = { | ||
|
|
||
| // Helper functions follow. | ||
|
|
||
| var UNDERSCORE_LOWERCASE_REGEXP = /_([a-z])/g; | ||
|
|
||
| function snakeToCamel(name) { | ||
| return name | ||
| .replace(UNDERSCORE_LOWERCASE_REGEXP, fnCamelCaseReplace); | ||
| } | ||
|
|
||
| function adjustMatcher(matcher) { | ||
| if (matcher === 'self') { | ||
| return matcher; | ||
| @@ -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); | ||
| }; | ||
| }); | ||
| @@ -118,7 +118,8 @@ | ||
| /* jqLite.js */ | ||
| "BOOLEAN_ATTR": false, | ||
| "jqNextId": false, | ||
| "camelCase": false, | ||
| "kebabToCamel": false, | ||
| "fnCamelCaseReplace": false, | ||
| "jqLitePatchJQueryRemove": false, | ||
| "JQLite": false, | ||
| "jqLiteClone": false, | ||
| @@ -594,6 +594,39 @@ describe('jqLite', function() { | ||
| }).not.toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('camelCasing keys', function() { | ||
| // jQuery 2.x has different behavior; skip the tests. | ||
| if (isJQuery2x()) return; | ||
|
|
||
| it('should camelCase the key in a setter', function() { | ||
| var element = jqLite(a); | ||
|
|
||
| element.data('a-B-c-d-42--e', 'z-x'); | ||
| expect(element.data()).toEqual({'a-BCD-42-E': 'z-x'}); | ||
| }); | ||
|
|
||
| it('should camelCase the key in a getter', function() { | ||
| var element = jqLite(a); | ||
|
|
||
| element.data()['a-BCD-42-E'] = 'x-c'; | ||
| expect(element.data('a-B-c-d-42--e')).toBe('x-c'); | ||
| }); | ||
|
|
||
| it('should camelCase the key in a mass setter', function() { | ||
| var element = jqLite(a); | ||
|
|
||
| element.data({'a-B-c-d-42--e': 'c-v', 'r-t-v': 42}); | ||
| expect(element.data()).toEqual({'a-BCD-42-E': 'c-v', 'rTV': 42}); | ||
| }); | ||
|
|
||
| it('should ignore non-camelCase keys in the data in a getter', function() { | ||
| var element = jqLite(a); | ||
|
|
||
| element.data()['a-b'] = 'b-n'; | ||
| expect(element.data('a-b')).toBe(undefined); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
| @@ -1055,6 +1088,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'); | ||
| }); | ||
|
|
||
| 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'); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
| @@ -2267,25 +2399,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'); | ||
| }); | ||
| }); | ||
|
|
||
Showing you all comments on commits in this comparison.
This comment has been minimized.
This comment has been minimized.
|
As @jbedard noticed, the before/after examples under BEFORE:
elem.data()['foo-bar'] = 1;
elem.data()['fooBar'] = 2;
elem.data('foo-bar'); // 1
AFTER:
elem.data()['foo-bar'] = 1;
elem.data()['fooBar'] = 2;
elem.data('foo-bar'); // 2The point is that |
This comment has been minimized.
This comment has been minimized.
StNekroman
commented on fc0c11d
Sep 28, 2018
|
very bad change |