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

Prevent mini size #89

Merged
merged 1 commit into from Aug 4, 2019

Conversation

@X-Ryl669
Copy link
Contributor

commented Jul 15, 2019

This should fix #88.

The idea here is to emit a warning whenever a selector is found in the compressed/renamed map and not in the uncompressed map (something is really broken if it's the case). It also prevent silent error by returning the selector appended with _conflict to avoid selecting the bad elements.

@coveralls

This comment has been minimized.

Copy link

commented Jul 15, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 26e6785 on X-Ryl669:PreventMiniSize into f7419e6 on JPeer264:master.

@X-Ryl669 X-Ryl669 force-pushed the X-Ryl669:PreventMiniSize branch from 7e4adec to b5319cd Jul 15, 2019

@JPeer264
Copy link
Owner

left a comment

Some changes came from another PR. I know now which changes should be in this PR, so it's ok - they will get merged anyways later on.

@@ -38,9 +40,19 @@ export class BaseLibrary {

const options = merge({}, optionsDefault, opts);

if (!this.values[value] && options.isOriginalValue && this.compressedValues[value]) {
// eslint-disable-next-line no-console
console.warn(`WARNING: '${value}' does not exist beforehand in the rename table,` +

This comment has been minimized.

Copy link
@JPeer264

JPeer264 Jul 23, 2019

Owner

As this code is a duplicate in selectorLibrary.js I think it is better to create a static method in BaseLibrary for the logs.

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.`);
	// eslint-disable-next-line no-console
	console.warn(`- Add the value to the reserved selectors table so it's not used for renaming.`);

	return `${value}_conflict`;
}

And later you could return this return BaseLibrary.hasReservedValue(value);. Or maybe you have a better idea :)

This comment has been minimized.

Copy link
@X-Ryl669

X-Ryl669 Aug 3, 2019

Author Contributor

Yes you're completely right. I'll do this.

This comment has been minimized.

Copy link
@X-Ryl669

X-Ryl669 Aug 4, 2019

Author Contributor

Done.

@@ -41,7 +41,24 @@ const replaceJs = (code, espreeOptions) => {

traverse(ast, {
pre: (node) => {
if (node.type === 'Literal' && typeof node.value === 'string') {
// Avoid recursing into a "in" node since it can't be a CSS class or variable.

This comment has been minimized.

Copy link
@JPeer264

JPeer264 Jul 23, 2019

Owner

This seems to come from another PR

This comment has been minimized.

Copy link
@X-Ryl669

X-Ryl669 Aug 3, 2019

Author Contributor

Oups. 😢
I was working on this at the same time as the cuckoo stuff, and I mixed my branches.

@X-Ryl669 X-Ryl669 force-pushed the X-Ryl669:PreventMiniSize branch from b5319cd to a348de1 Aug 4, 2019

@X-Ryl669

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2019

Should be better now.

@X-Ryl669 X-Ryl669 force-pushed the X-Ryl669:PreventMiniSize branch from a348de1 to 26e6785 Aug 4, 2019

@JPeer264 JPeer264 merged commit 10ddbd9 into JPeer264:master Aug 4, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.