From 2e3a45b95c55ca5725ce78dca9d4ab0779f5c84c Mon Sep 17 00:00:00 2001 From: Labayna Neil Brian Narido Date: Wed, 26 Feb 2020 23:34:32 +0800 Subject: [PATCH 1/2] Add warnings for inconsistent/deprecated component attribs Use console log instead of logger add warning for modal slots Rewrite more efficient methods and use element Rewrite docs and reduce obvious code Reduce obvious code Remove empty lines Add gaurd clause and remove excessive todo Change console log to warn and remove return Change console log to warn Add more warns for dropdown Use conflicting instead of inconsistent Change c to child Add note regarding conlifcting attribs Restructure warnDeprecatedAttributes Use Foreach instead of loop Group all warns and put at start of parsing methods Restructure warnDeprecatedSlotName --- .../markbind/src/parsers/componentParser.js | 101 ++++++++++++++++-- 1 file changed, 91 insertions(+), 10 deletions(-) diff --git a/src/lib/markbind/src/parsers/componentParser.js b/src/lib/markbind/src/parsers/componentParser.js index 7b7ef582cb..1d619ce14a 100644 --- a/src/lib/markbind/src/parsers/componentParser.js +++ b/src/lib/markbind/src/parsers/componentParser.js @@ -134,6 +134,45 @@ function _assignPanelId(node) { } } +/** + * Check and warns if element has conflicting attributes. + * Note that attirbutes in `attrsConflictingWith` are not in conflict with each other, + * but only towards `attribute` + * @param node Root element to check + * @param attribute An attribute that is conflicting with other attributes + * @param attrsConflictingWith The attributes conflicting with `attribute` + */ +function _warnConflictingAttributes(node, attribute, attrsConflictingWith) { + if (!(attribute in node.attribs)) { + return; + } + attrsConflictingWith.forEach((conflictingAttr) => { + if (conflictingAttr in node.attribs) { + // TODO: Use logger here instead of console.warn. See issue #1060 + // eslint-disable-next-line no-console + console.warn(`warn: Usage of conflicting ${node.name} attributes: ` + + `'${attribute}' with '${conflictingAttr}'`); + } + }); +} + +/** + * Check and warns if element has a deprecated attribute. + * @param node Root element to check + * @param attributeNamePairs Object of attribute name pairs with each pair in the form deprecated : correct + */ +function _warnDeprecatedAttributes(node, attributeNamePairs) { + Object.entries(attributeNamePairs).forEach(([deprecatedAttrib, correctAttrib]) => { + if (deprecatedAttrib in node.attribs) { + // TODO: Use logger here instead of console.warn. See issue #1060 + // eslint-disable-next-line no-console + console.warn(`warn: ${node.name} attribute '${deprecatedAttrib}' ` + + `is deprecated and may be removed in the future. Please use '${correctAttrib}'`); + } + }); +} + + /* * Triggers * @@ -171,9 +210,10 @@ function _parseTrigger(node) { */ function _parsePopover(node) { + _warnDeprecatedAttributes(node, { title: 'header' }); + _parseAttributeWithoutOverride(node, 'content', true); _parseAttributeWithoutOverride(node, 'header', true); - // TODO deprecate title attribute for popovers _parseAttributeWithoutOverride(node, 'title', true, 'header'); node.name = 'span'; @@ -186,6 +226,32 @@ function _parsePopover(node) { _transformSlottedComponents(node); } +/** + * Check and warns if element has a deprecated slot name + * @param element Root element to check + * @param namePairs Object of slot name pairs with each pair in the form deprecated : correct + */ + +function _warnDeprecatedSlotNames(element, namePairs) { + if (!(element.children)) { + return; + } + element.children.forEach((child) => { + if (!(_.has(child.attribs, 'slot'))) { + return; + } + Object.entries(namePairs).forEach(([deprecatedName, correctName]) => { + if (child.attribs.slot !== deprecatedName) { + return; + } + // TODO: Use logger here instead of console.warn. See issue #1060 + // eslint-disable-next-line no-console + console.warn(`warn: ${element.name} slot name '${deprecatedName}' ` + + `is deprecated and may be removed in the future. Please use '${correctName}'`); + }); + }); +} + /* * Tooltips * @@ -207,7 +273,8 @@ function _parseTooltip(node) { function _renameSlot(node, originalName, newName) { if (node.children) { - node.children.forEach((child) => { + node.children.forEach((c) => { + const child = c; if (_.has(child.attribs, 'slot') && child.attribs.slot === originalName) { child.attribs.slot = newName; } @@ -230,11 +297,12 @@ function _renameAttribute(node, originalAttribute, newAttribute) { */ function _parseModalAttributes(node) { + _warnDeprecatedAttributes(node, { title: 'header' }); + _warnDeprecatedSlotNames(node, { 'modal-header': 'header', 'modal-footer': 'footer' }); + _parseAttributeWithoutOverride(node, 'header', true, 'modal-title'); - // TODO deprecate title attribute for modals _parseAttributeWithoutOverride(node, 'title', true, 'modal-title'); - // TODO deprecate modal-header, modal-footer attributes for modals _renameSlot(node, 'header', 'modal-header'); _renameSlot(node, 'footer', 'modal-footer'); @@ -282,13 +350,16 @@ function _parseTabAttributes(node) { */ function _parseBoxAttributes(node) { + _warnConflictingAttributes(node, 'light', ['seamless']); + _warnConflictingAttributes(node, 'no-background', ['background-color', 'seamless']); + _warnConflictingAttributes(node, 'no-border', ['border-color', 'border-left-color', 'seamless']); + _warnConflictingAttributes(node, 'no-icon', ['icon']); + _warnDeprecatedAttributes(node, { heading: 'header' }); + _parseAttributeWithoutOverride(node, 'icon', true, '_icon'); _parseAttributeWithoutOverride(node, 'header', false, '_header'); - // TODO deprecate heading attribute for box _parseAttributeWithoutOverride(node, 'heading', false, '_header'); - - // TODO warn when light and seamless attributes are both present } /* @@ -301,18 +372,28 @@ function _parseDropdownAttributes(node) { // If header slot is present, the header attribute has no effect, and we can simply remove it. if (hasHeaderSlot) { + if (_.has(node.attribs, 'header')) { + // TODO: Use logger here instead of console.warn. See issue #1060 + // eslint-disable-next-line no-console + console.warn(`warn: ${node.name} has a header slot, 'header' attribute has no effect.`); + } + if (_.has(node.attribs, 'text')) { + // TODO: Use logger here instead of console.warn. See issue #1060 + // eslint-disable-next-line no-console + console.warn(`warn: ${node.name} has a header slot, 'text' attribute has no effect.`); + } delete node.attribs.header; - // TODO deprecate text attribute of dropdown delete node.attribs.text; return; } - // header attribute takes priority over text attribute + _warnDeprecatedAttributes(node, { text: 'header' }); + _warnConflictingAttributes(node, 'header', ['text']); + // header attribute takes priority over text attribute if both 'text' and 'header' is used if (_.has(node.attribs, 'header')) { _parseAttributeWithoutOverride(node, 'header', true, '_header'); delete node.attribs.text; } else { - // TODO deprecate text attribute of dropdown _parseAttributeWithoutOverride(node, 'text', true, '_header'); } } From de67b192cc0053d2a23c2f49f17a9538066c86c6 Mon Sep 17 00:00:00 2001 From: Labayna Neil Brian Narido Date: Sat, 7 Mar 2020 15:54:00 +0800 Subject: [PATCH 2/2] linting --- src/lib/markbind/src/parsers/componentParser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/markbind/src/parsers/componentParser.js b/src/lib/markbind/src/parsers/componentParser.js index 1d619ce14a..c9ed5eead3 100644 --- a/src/lib/markbind/src/parsers/componentParser.js +++ b/src/lib/markbind/src/parsers/componentParser.js @@ -299,7 +299,7 @@ function _renameAttribute(node, originalAttribute, newAttribute) { function _parseModalAttributes(node) { _warnDeprecatedAttributes(node, { title: 'header' }); _warnDeprecatedSlotNames(node, { 'modal-header': 'header', 'modal-footer': 'footer' }); - + _parseAttributeWithoutOverride(node, 'header', true, 'modal-title'); _parseAttributeWithoutOverride(node, 'title', true, 'modal-title');