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.

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

Better error reporting #94

Merged
merged 3 commits into from Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 69 additions & 0 deletions lib/allWarnings.js
@@ -0,0 +1,69 @@
export class Warnings {
constructor() {
this.reset();
}

summary(text) {
if (text.length > 120) {
// eslint-disable-next-line no-param-reassign
this.ranOnMinifiedFiles = this.ranOnMinifiedFiles || text.length > 500;
return `${text.slice(0, 120)}...`;
}
return text;
}

reset() {
this.warningArray = {};
this.ranOnMinifiedFiles = false;
}
append(value, source) {
if (value in this.warningArray) {
if (source && this.warningArray[value].findIndex(
e => e.file === source.file && e.line === source.line) === -1
) {
this.warningArray[value].push(source);
}
} else {
this.warningArray[value] = source ? [source] : [];
}
}
warn() {
const keys = Object.keys(this.warningArray);
if (!keys.length) return;

// eslint-disable-next-line no-console
console.warn('WARNING: The following selectors were not found in the rename table, but ' +
'appears in the compressed map. In order to avoid that some other selectors ' +
'are used instead, they were appended with \'_conflict\'. You need to fix this ' +
'either by:\n');
// eslint-disable-next-line no-console
console.warn('- Creating a CSS rule with the selector name and re-run the process, or');
// eslint-disable-next-line no-console
console.warn('- Excluding the selectors so it\'s not renamed, or');
// eslint-disable-next-line no-console
console.warn('- Adding the value to the reserved selectors table so it\'s not used as a possible short name\n\n');

// eslint-disable-next-line no-console
console.warn('The failing selector are:');
keys.forEach((key) => {
const line = this.warningArray[key];
if (line.length) {
// eslint-disable-next-line no-console
console.warn(` - '${key}' found in: `);
// eslint-disable-next-line no-console
line.forEach(e => console.warn(` ${e.file}(${e.line}): ${this.summary(e.text)}`));
} else {
// eslint-disable-next-line no-console
console.warn(` - '${key}'`);
}
});

if (this.ranOnMinifiedFiles) {
// eslint-disable-next-line no-console
console.warn('WARNING: You shouldn\'t run this software on minified files as it\'ll be ' +
'hard to debug errors whenever they happens.\n');
}
}
}

export default new Warnings();
13 changes: 4 additions & 9 deletions lib/baseLibrary.js
Expand Up @@ -3,6 +3,7 @@ import entries from 'object.entries';
import merge from 'lodash.merge';

import { NameGenerator } from './nameGenerator';
import warnings from './allWarnings';

export class BaseLibrary {
constructor(name) {
Expand Down Expand Up @@ -54,14 +55,8 @@ export class BaseLibrary {
}


static hasReservedValue(value) {
// eslint-disable-next-line no-console
console.warn(`WARNING: '${value}' does not exist beforehand in the rename table,` +
' but appears in the compressed map. Either:');
// eslint-disable-next-line no-console
console.warn(`- Create a CSS rule with '${value}' and re-run the process, or`);
// eslint-disable-next-line no-console
console.warn('- Add the value to the reserved selectors table so it\'s not used for renaming');
static hasReservedValue(value, source) {
warnings.append(value, source);
return `${value}_conflict`;
} // /hasReservedValue

Expand All @@ -81,7 +76,7 @@ export class BaseLibrary {
}

if (!this.values[finalValue] && options.isOriginalValue && this.compressedValues[finalValue]) {
return BaseLibrary.hasReservedValue(value);
return BaseLibrary.hasReservedValue(value, options.source);
}

let found = finalValue in this.values;
Expand Down
2 changes: 2 additions & 0 deletions lib/index.js
Expand Up @@ -9,6 +9,7 @@ import cssVariablesLibrary from './cssVariablesLibrary';

import extractFromHtml from './helpers/extractFromHtml';
import htmlToAst from './helpers/htmlToAst';
import warnings from './allWarnings';

export default {
stats,
Expand All @@ -23,4 +24,5 @@ export default {
htmlToAst,
extractFromHtml,
},
warnings,
};
4 changes: 1 addition & 3 deletions lib/nameGenerator.js
Expand Up @@ -2,9 +2,7 @@ import decToAny from 'decimal-to-any';

let customGenerator = null;

function defaultGenerator(obj) {
return decToAny(obj.nameCounter, obj.alphabet.length, obj);
}
const defaultGenerator = obj => decToAny(obj.nameCounter, obj.alphabet.length, obj);

export class NameGenerator {
constructor(type) {
Expand Down
4 changes: 2 additions & 2 deletions lib/replace/any.js
Expand Up @@ -2,7 +2,7 @@ import selectorsLibrary from '../selectorsLibrary';
import replaceString from './string';
import replaceRegex from './regex';

const replaceAny = (code) => {
const replaceAny = (code, opts = {}) => {
const regex = selectorsLibrary.getAllRegex();

let data = code.toString();
Expand All @@ -11,7 +11,7 @@ const replaceAny = (code) => {
return data;
}

data = data.replace(replaceRegex.strings, match => replaceString(match, regex));
data = data.replace(replaceRegex.strings, match => replaceString(match, regex, ' ', opts));

return data;
};
Expand Down
18 changes: 13 additions & 5 deletions lib/replace/css.js
Expand Up @@ -55,7 +55,7 @@ const getAttributeSelector = (match) => {
return result;
}; // /getCssSelector

const replaceCss = (css) => {
const replaceCss = (css, opts = {}) => {
const cssAST = parse(css);

/* ******************** *
Expand All @@ -64,6 +64,7 @@ const replaceCss = (css) => {
cssAST.walk((node) => {
const parentName = node.parent.name || '';
const selectorLib = selectorsLibrary.getIdSelector();
const source = { file: opts.sourceFile, line: node.source.start.line, text: '' };

if (node.selector && !parentName.match(/keyframes/)) {
const regex = selectorLib.getAll({ regex: true, addSelectorType: true });
Expand All @@ -74,6 +75,7 @@ const replaceCss = (css) => {
return prefixFreeSelector.replace(regex, match => (
selectorLib.get(match, {
addSelectorType: true,
source,
})
));
});
Expand All @@ -86,6 +88,7 @@ const replaceCss = (css) => {
cssAST.walk((node) => {
const parentName = node.parent.name || '';
const selectorLib = selectorsLibrary.getClassSelector();
const source = { file: opts.sourceFile, line: node.source.start.line, text: '' };

if (node.selector && !parentName.match(/keyframes/)) {
const regex = selectorLib.getAll({ regex: true, addSelectorType: true });
Expand All @@ -96,6 +99,7 @@ const replaceCss = (css) => {
return prefixFreeSelector.replace(regex, match => (
selectorLib.get(match, {
addSelectorType: true,
source,
})
));
});
Expand All @@ -111,12 +115,16 @@ const replaceCss = (css) => {
return;
}

const source = { file: opts.sourceFile, line: node.source.start.line, text: '' };

// do not count stats, as these are just the declarations
// eslint-disable-next-line no-param-reassign
node.params = keyframesLibrary.get(node.params, { countStats: false });
node.params = keyframesLibrary.get(node.params, { countStats: false, source });
});

cssAST.walkDecls((node) => {
const source = { file: opts.sourceFile, line: node.source.start.line, text: '' };

/* ************************** *
* replace css variables var() *
* *************************** */
Expand All @@ -125,7 +133,7 @@ const replaceCss = (css) => {

// eslint-disable-next-line no-param-reassign
node.value = node.value.replace(new RegExp(matches.join('|'), 'g'), match => (
cssVariablesLibrary.get(match)
cssVariablesLibrary.get(match, { source })
));
}

Expand All @@ -135,7 +143,7 @@ const replaceCss = (css) => {
if (node.type === 'decl' && node.prop.match('^--')) {
// do not count stats, as these are just the declarations
// eslint-disable-next-line no-param-reassign
node.prop = cssVariablesLibrary.get(node.prop, { countStats: false });
node.prop = cssVariablesLibrary.get(node.prop, { countStats: false, source });
}

/* ***************** *
Expand All @@ -147,7 +155,7 @@ const replaceCss = (css) => {
.replace(',', ' , ')
.split(' ')
.map(value => (
keyframesLibrary.get(value)
keyframesLibrary.get(value, { source })
))
.join(' ')
.replace(' , ', ',');
Expand Down
13 changes: 10 additions & 3 deletions lib/replace/html.js
Expand Up @@ -22,6 +22,7 @@ const replaceHtml = (code, opts = {}) => {

const options = merge(opts, defaultOptions);
const ast = htmlToAst(code);
const srcOpt = { sourceFile: opts.sourceFile };

traverse(ast, {
pre: (node) => {
Expand All @@ -47,14 +48,14 @@ const replaceHtml = (code, opts = {}) => {
// type set to application/json || module
if (hasAnyAttrs || !hasType || hasTypeAndJavaScript) {
// eslint-disable-next-line no-param-reassign
node.value = replaceJs(node.value, options.espreeOptions);
node.value = replaceJs(node.value, merge(options.espreeOptions, srcOpt));
}
}

// rename <style> tags
if (node.parentNode && node.parentNode.tagName === 'style') {
// eslint-disable-next-line no-param-reassign
node.value = replaceCss(node.value);
node.value = replaceCss(node.value, srcOpt);
}

// rename attributes
Expand Down Expand Up @@ -93,7 +94,13 @@ const replaceHtml = (code, opts = {}) => {
.map(value => (
// renaming each value
selectorsLibrary
.get(`${selectorType}${value}`)
.get(`${selectorType}${value}`, {
source: {
file: opts.sourceFile,
line: node.sourceCodeLocation.startLine,
text: '',
},
})
.replace(new RegExp(`^\\${selectorType}`), '')
))
.join(' ');
Expand Down
18 changes: 14 additions & 4 deletions lib/replace/js.js
Expand Up @@ -8,6 +8,14 @@ import replaceString from './string';
import cssVariablesLibrary from '../cssVariablesLibrary';
import allRegex from './regex';

const makeSource = (node, file) => {
const Position = node.loc.start.constructor;
const currentLine = node.loc.start.line;
const sourceLine = node.loc.lines.sliceString(new Position(currentLine, 0),
new Position(currentLine, node.loc.lines.getLineLength(currentLine)));
return { file, line: currentLine, text: sourceLine };
};

const replaceJs = (code, espreeOptions) => {
// We can only use the common regex if we don't care about specific class/id processing
const regex = selectorsLibrary.getAllRegex();
Expand Down Expand Up @@ -46,32 +54,34 @@ const replaceJs = (code, espreeOptions) => {
}

if (node.type === 'TemplateElement' && 'raw' in node.value) {
const source = makeSource(node, options.sourceFile);
// eslint-disable-next-line
const raw = node.value.raw.replace(allRegex.templateSelectors, function (txt, p1, p2, p3) {
// p3 contains the content of the class=' or id=", so let's replace them
const newValue = ` ${p3} `;
const selectorLib = p1 === 'class' ? selectorsLibrary.getClassSelector()
: selectorsLibrary.getIdSelector();
const replacedAttr = newValue.replace(newValue, match =>
replaceString(match, selectorLib.getAll({ regex: true }), ' ', { countStats: false }));
replaceString(match, selectorLib.getAll({ regex: true }), ' ', { countStats: false, source }));
// eslint-disable-next-line
return p1 + '=' + p2 + replacedAttr.slice(1, replacedAttr.length - 1) + p2;
});

// eslint-disable-next-line no-param-reassign
node.value.raw = raw;
} else if (node.type === 'Literal' && typeof node.value === 'string') {
const source = makeSource(node, options.sourceFile);
// eslint-disable-next-line no-param-reassign
node.raw = node.raw.replace(node.raw, match => replaceString(match, regex));
node.raw = node.raw.replace(node.raw, match => replaceString(match, regex, ' ', { source }));

// add whitespaces before and after
// to make the regex work
const newValue = ` ${node.value} `;
// replace css selectors
const replacedCssSelectors = newValue.replace(newValue, match => replaceString(match, regex, ' ', { countStats: false }));
const replacedCssSelectors = newValue.replace(newValue, match => replaceString(match, regex, ' ', { countStats: false, source }));
// replace css variables
const replacedCssVariables = replacedCssSelectors.replace(allRegex.cssVariables, match => (
cssVariablesLibrary.get(match)
cssVariablesLibrary.get(match, { source })
));

// eslint-disable-next-line no-param-reassign
Expand Down
8 changes: 5 additions & 3 deletions lib/replace/pug.js
Expand Up @@ -47,8 +47,8 @@ const replacePug = (code, opts = {}) => {
))
))();
const replacedCode = node.name === 'script'
? replaceJs(newCode, options.espreeOptions)
: replaceCss(newCode);
? replaceJs(newCode, merge(options.espreeOptions, { sourceFile: opts.sourceFile }))
: replaceCss(newCode, { sourceFile: opts.sourceFile });

// add one tab after each new line
const pugCode = `${node.name}.\n${replacedCode}`.replace(/\n/g, '\n\t');
Expand Down Expand Up @@ -108,7 +108,9 @@ const replacePug = (code, opts = {}) => {
.map(value => (
// renaming each value
selectorsLibrary
.get(`${selectorType}${value}`)
.get(`${selectorType}${value}`, {
source: { file: opts.sourceFile, line: node.line, text: '' },
})
.replace(new RegExp(`^\\${selectorType}`), '')
))
.join(' ');
Expand Down