Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix whitespace bug in x-bind:class class list #437

Merged
merged 3 commits into from
May 8, 2020
Merged

Fix whitespace bug in x-bind:class class list #437

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce51161
remove empty classes created by whitespaces
felixsc Apr 29, 2020
180ab28
test whitespace removal in class list
felixsc Apr 29, 2020
35ec9db
re-build dist
felixsc Apr 30, 2020
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
92 changes: 53 additions & 39 deletions dist/alpine-ie11.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@
(module.exports = function (key, value) {
return sharedStore[key] || (sharedStore[key] = value !== undefined ? value : {});
})('versions', []).push({
version: '3.6.4',
version: '3.6.5',
mode: 'global',
copyright: '© 2020 Denis Pushkarev (zloirock.ru)'
});
Expand Down Expand Up @@ -2348,23 +2348,36 @@
}

function _toConsumableArray(arr) {
return _arrayWithoutHoles(arr) || _iterableToArray(arr) || _nonIterableSpread();
return _arrayWithoutHoles(arr) || _iterableToArray(arr) || _unsupportedIterableToArray(arr) || _nonIterableSpread();
}

function _arrayWithoutHoles(arr) {
if (Array.isArray(arr)) {
for (var i = 0, arr2 = new Array(arr.length); i < arr.length; i++) arr2[i] = arr[i];

return arr2;
}
if (Array.isArray(arr)) return _arrayLikeToArray(arr);
}

function _iterableToArray(iter) {
if (Symbol.iterator in Object(iter) || Object.prototype.toString.call(iter) === "[object Arguments]") return Array.from(iter);
if (typeof Symbol !== "undefined" && Symbol.iterator in Object(iter)) return Array.from(iter);
}

function _unsupportedIterableToArray(o, minLen) {
if (!o) return;
if (typeof o === "string") return _arrayLikeToArray(o, minLen);
var n = Object.prototype.toString.call(o).slice(8, -1);
if (n === "Object" && o.constructor) n = o.constructor.name;
if (n === "Map" || n === "Set") return Array.from(o);
if (n === "Arguments" || /^(?:Ui|I)nt(?:8|16|32)(?:Clamped)?Array$/.test(n)) return _arrayLikeToArray(o, minLen);
}

function _arrayLikeToArray(arr, len) {
if (len == null || len > arr.length) len = arr.length;

for (var i = 0, arr2 = new Array(len); i < len; i++) arr2[i] = arr[i];

return arr2;
}

function _nonIterableSpread() {
throw new TypeError("Invalid attempt to spread non-iterable instance");
throw new TypeError("Invalid attempt to spread non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");
}

var runtime_1 = createCommonjsModule(function (module) {
Expand Down Expand Up @@ -3466,7 +3479,13 @@
defer = functionBindContext(port.postMessage, port, 1);
// Browsers with postMessage, skip WebWorkers
// IE8 has postMessage, but it's sync & typeof its postMessage is 'object'
} else if (global_1.addEventListener && typeof postMessage == 'function' && !global_1.importScripts && !fails(post)) {
} else if (
global_1.addEventListener &&
typeof postMessage == 'function' &&
!global_1.importScripts &&
!fails(post) &&
location.protocol !== 'file:'
) {
defer = post;
global_1.addEventListener('message', listener, false);
// IE8-
Expand Down Expand Up @@ -6022,13 +6041,13 @@
_newArrowCheck(this, _this);

if (value[classNames]) {
classNames.split(' ').forEach(function (className) {
classNames.split(' ').filter(Boolean).forEach(function (className) {
_newArrowCheck(this, _this2);

return el.classList.add(className);
}.bind(this));
} else {
classNames.split(' ').forEach(function (className) {
classNames.split(' ').filter(Boolean).forEach(function (className) {
_newArrowCheck(this, _this2);

return el.classList.remove(className);
Expand All @@ -6038,7 +6057,7 @@
} else {
var _originalClasses = el.__x_original_classes || [];

var newClasses = value.split(' ');
var newClasses = value.split(' ').filter(Boolean);
el.setAttribute('class', arrayUnique(_originalClasses.concat(newClasses)).join(' '));
}
} else if (isBooleanAttr(attrName)) {
Expand Down Expand Up @@ -6278,7 +6297,7 @@
return component.evaluateCommandExpression(e.target, expression, function () {
_newArrowCheck(this, _this2);

return _objectSpread2({}, extraVars(), {
return _objectSpread2(_objectSpread2({}, extraVars()), {}, {
'$event': e
});
}.bind(this));
Expand Down Expand Up @@ -6362,7 +6381,7 @@
registerListener(component, el, event, modifiers, listenerExpression, function () {
_newArrowCheck(this, _this);

return _objectSpread2({}, extraVars(), {
return _objectSpread2(_objectSpread2({}, extraVars()), {}, {
rightSideOfExpression: generateModelAssignmentFunction(el, modifiers, expression)
});
}.bind(this));
Expand Down Expand Up @@ -6812,13 +6831,13 @@
var _this14 = this;

getXAttrs(el).forEach(function (_ref) {
_newArrowCheck(this, _this14);

var type = _ref.type,
value = _ref.value,
modifiers = _ref.modifiers,
expression = _ref.expression;

_newArrowCheck(this, _this14);

switch (type) {
case 'on':
registerListener(this, el, value, modifiers, expression, extraVars);
Expand Down Expand Up @@ -6856,13 +6875,13 @@
attrs.forEach(function (_ref2) {
var _this16 = this;

_newArrowCheck(this, _this15);

var type = _ref2.type,
value = _ref2.value,
modifiers = _ref2.modifiers,
expression = _ref2.expression;

_newArrowCheck(this, _this15);

switch (type) {
case 'model':
handleAttributeBindingDirective(this, el, 'value', expression, extraVars, type);
Expand Down Expand Up @@ -6918,7 +6937,7 @@
var extraVars = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : function () {
_newArrowCheck(this, _this17);
}.bind(this);
return saferEval(expression, this.$data, _objectSpread2({}, extraVars(), {
return saferEval(expression, this.$data, _objectSpread2(_objectSpread2({}, extraVars()), {}, {
$dispatch: this.getDispatchFunction(el)
}));
}
Expand All @@ -6930,30 +6949,25 @@
var extraVars = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : function () {
_newArrowCheck(this, _this18);
}.bind(this);
return saferEvalNoReturn(expression, this.$data, _objectSpread2({}, extraVars(), {
return saferEvalNoReturn(expression, this.$data, _objectSpread2(_objectSpread2({}, extraVars()), {}, {
$dispatch: this.getDispatchFunction(el)
}));
}
}, {
key: "getDispatchFunction",
value: function getDispatchFunction(el) {
var _this19 = this;

return function (event) {
var detail = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {};

_newArrowCheck(this, _this19);

el.dispatchEvent(new CustomEvent(event, {
detail: detail,
bubbles: true
}));
}.bind(this);
};
}
}, {
key: "listenForNewElementsToInitialize",
value: function listenForNewElementsToInitialize() {
var _this20 = this;
var _this19 = this;

var targetNode = this.$el;
var observerOptions = {
Expand All @@ -6962,9 +6976,9 @@
subtree: true
};
var observer = new MutationObserver(function (mutations) {
var _this21 = this;
var _this20 = this;

_newArrowCheck(this, _this20);
_newArrowCheck(this, _this19);

for (var i = 0; i < mutations.length; i++) {
// Filter out mutations triggered from child components.
Expand All @@ -6973,22 +6987,22 @@

if (mutations[i].type === 'attributes' && mutations[i].attributeName === 'x-data') {
(function () {
var _this22 = this;
var _this21 = this;

var rawData = saferEval(mutations[i].target.getAttribute('x-data'), {});
Object.keys(rawData).forEach(function (key) {
_newArrowCheck(this, _this22);
_newArrowCheck(this, _this21);

if (_this21.$data[key] !== rawData[key]) {
_this21.$data[key] = rawData[key];
if (_this20.$data[key] !== rawData[key]) {
_this20.$data[key] = rawData[key];
}
}.bind(this));
})();
}

if (mutations[i].addedNodes.length > 0) {
mutations[i].addedNodes.forEach(function (node) {
_newArrowCheck(this, _this21);
_newArrowCheck(this, _this20);

if (node.nodeType !== 1 || node.__x_inserted_me) return;

Expand All @@ -7007,7 +7021,7 @@
}, {
key: "getRefsProxy",
value: function getRefsProxy() {
var _this23 = this;
var _this22 = this;

var self = this;
var refObj = {};
Expand All @@ -7019,7 +7033,7 @@
// we just loop on the element, look for any x-ref and create a tmp property on a fake object.

this.walkAndSkipNestedComponents(self.$el, function (el) {
_newArrowCheck(this, _this23);
_newArrowCheck(this, _this22);

if (el.hasAttribute('x-ref')) {
refObj[el.getAttribute('x-ref')] = true;
Expand All @@ -7033,14 +7047,14 @@

return new Proxy(refObj, {
get: function get(object, property) {
var _this24 = this;
var _this23 = this;

if (property === '$isAlpineProxy') return true;
var ref; // We can't just query the DOM because it's hard to filter out refs in
// nested components.

self.walkAndSkipNestedComponents(self.$el, function (el) {
_newArrowCheck(this, _this24);
_newArrowCheck(this, _this23);

if (el.hasAttribute('x-ref') && el.getAttribute('x-ref') === property) {
ref = el;
Expand Down
14 changes: 7 additions & 7 deletions dist/alpine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,14 +577,14 @@
const keysSortedByBooleanValue = Object.keys(value).sort((a, b) => value[a] - value[b]);
keysSortedByBooleanValue.forEach(classNames => {
if (value[classNames]) {
classNames.split(' ').forEach(className => el.classList.add(className));
classNames.split(' ').filter(Boolean).forEach(className => el.classList.add(className));
} else {
classNames.split(' ').forEach(className => el.classList.remove(className));
classNames.split(' ').filter(Boolean).forEach(className => el.classList.remove(className));
}
});
} else {
const originalClasses = el.__x_original_classes || [];
const newClasses = value.split(' ');
const newClasses = value.split(' ').filter(Boolean);
el.setAttribute('class', arrayUnique(originalClasses.concat(newClasses)).join(' '));
}
} else if (isBooleanAttr(attrName)) {
Expand Down Expand Up @@ -772,7 +772,7 @@

function runListenerHandler(component, expression, e, extraVars) {
return component.evaluateCommandExpression(e.target, expression, () => {
return _objectSpread2({}, extraVars(), {
return _objectSpread2(_objectSpread2({}, extraVars()), {}, {
'$event': e
});
});
Expand Down Expand Up @@ -838,7 +838,7 @@
var event = el.tagName.toLowerCase() === 'select' || ['checkbox', 'radio'].includes(el.type) || modifiers.includes('lazy') ? 'change' : 'input';
const listenerExpression = `${expression} = rightSideOfExpression($event, ${expression})`;
registerListener(component, el, event, modifiers, listenerExpression, () => {
return _objectSpread2({}, extraVars(), {
return _objectSpread2(_objectSpread2({}, extraVars()), {}, {
rightSideOfExpression: generateModelAssignmentFunction(el, modifiers, expression)
});
});
Expand Down Expand Up @@ -1546,13 +1546,13 @@
}

evaluateReturnExpression(el, expression, extraVars = () => {}) {
return saferEval(expression, this.$data, _objectSpread2({}, extraVars(), {
return saferEval(expression, this.$data, _objectSpread2(_objectSpread2({}, extraVars()), {}, {
$dispatch: this.getDispatchFunction(el)
}));
}

evaluateCommandExpression(el, expression, extraVars = () => {}) {
return saferEvalNoReturn(expression, this.$data, _objectSpread2({}, extraVars(), {
return saferEvalNoReturn(expression, this.$data, _objectSpread2(_objectSpread2({}, extraVars()), {}, {
$dispatch: this.getDispatchFunction(el)
}));
}
Expand Down
6 changes: 3 additions & 3 deletions src/directives/bind.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ export function handleAttributeBindingDirective(component, el, attrName, express

keysSortedByBooleanValue.forEach(classNames => {
if (value[classNames]) {
classNames.split(' ').forEach(className => el.classList.add(className))
classNames.split(' ').filter(Boolean).forEach(className => el.classList.add(className))
} else {
classNames.split(' ').forEach(className => el.classList.remove(className))
classNames.split(' ').filter(Boolean).forEach(className => el.classList.remove(className))
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to have a getClassNames util/helper that does the split + filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, makes sense. It's repeated three times in this PR

}
})
} else {
const originalClasses = el.__x_original_classes || []
const newClasses = value.split(' ')
const newClasses = value.split(' ').filter(Boolean)
el.setAttribute('class', arrayUnique(originalClasses.concat(newClasses)).join(' '))
}
} else if (isBooleanAttr(attrName)) {
Expand Down
11 changes: 11 additions & 0 deletions test/bind.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,3 +420,14 @@ test('setSelectionRange is not called for inapplicable input types', async () =>
expect(document.querySelector('input').value).toEqual('baz')
})
})

test('unnecessary whitespaces in class list are ignored', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to have tests for when it's using array syntax or string syntax?

document.body.innerHTML = `
<div x-data="{ foo: 'bar' }">
<span x-bind:class="{' foo:class ': foo === 'bar' }"></span>
</div>
`
Alpine.start()

expect(document.querySelector('span').classList.contains('foo:class')).toBeTruthy()
})