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

Crash with animation properties #941

Closed
mikestreety opened this issue May 10, 2017 · 8 comments
Closed

Crash with animation properties #941

mikestreety opened this issue May 10, 2017 · 8 comments
Labels
Milestone

Comments

@mikestreety
Copy link

Hello.

When I use animation & animation-* properties together, I get a crash on level 2 optimisations. I currently use gulp-clean-css but this seems like an isse with clean-css itself. It looks similar to #940

This error can be skipped by combing the animation properties for now.

Error:

Expand to see the error
/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/properties/every-values-pair.js:4
                     ^


TypeError: Cannot read property 'value' of undefined
    at everyValuesPair (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/properties/every-values-pair.js:4:22)
    at overrideProperties (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/properties/override-properties.js:367:13)
    at optimizeProperties (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/properties/optimize.js:33:5)
    at recursivelyOptimizeProperties (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/optimize.js:63:9)
    at level2Optimize (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/optimize.js:77:3)
    at recursivelyOptimizeBlocks (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/optimize.js:52:7)
    at level2Optimize (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/optimizer/level-2/optimize.js:76:3)
    at optimize (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/clean.js:131:5)
    at /usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:26:64
    at loadOriginalSources (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/load-original-sources.js:26:5)
    at loadOriginalSources (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/load-original-sources.js:26:5)
    at /usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:26:14
    at applySourceMaps (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/apply-source-maps.js:34:5)
    at Object.callback (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:25:12)
    at doInlineImports (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:200:25)
    at fromStyles (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:151:5)
    at fromString (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:48:10)
    at readSources (/usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/reader/read-sources.js:24:10)
    at /usr/lib/node_modules/gulp-clean-css/node_modules/clean-css/lib/clean.js:99:12
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)

Environment

Expand to see the environment
  • clean-css version - npm ls clean-css:
+-- gulp-clean-css@3.2.0
| `-- clean-css@4.1.1
  • node.js version - node -v:
v6.10.3
  • operating system:
Debian GNU/Linux 8 (jessie)

Configuration options

{
	level: {
		1: {
			inline: false,
			selectorsSortingMethod: false,
			specialComments: false,
		},
		2: {
			mergeAdjacentRules: true,
			mergeNonAdjacentRules: false,
			mergeIntoShorthands: false,
			removeDuplicateRules: true
		}
	}
}

Adding skipProperties: ['animation'] does not work

Input CSS

/* the shorter the better! */
li {
	animation: introGrow 0.4s ease-in;
	animation-delay: 0.2s;
}

animation-delay is calculated by SCSS and process before being run through clean-css. The reason this is split out is for readability.

Actual output CSS

None, it crashes

Expected output CSS

li:nth-child(#{$i}) {
	animation: introGrow 0.4s ease-in;
	animation-delay: 0.2s;
}
@jakubpawlowicz
Copy link
Collaborator

Hi @mikestreety - this is an output I get with 4.1.1:

li{animation:.4s ease-in .2s introGrow}

Could the problem lie somewhere else?
Could you also check with skipProperties: ['animation', 'animation-delay']?

@mikestreety
Copy link
Author

@jakubpawlowicz Thanks for the quick reply. I tried the skipProperties (placing after removeDuplicateRules) with the same result.

It's interesting you get that output. I've had the same thing on a different project, with the same animation property problem.

@jakubpawlowicz
Copy link
Collaborator

You should be able to workaround the issue by using overrideProperties: false as that's where the bug happens.

Would you be able to share, either publicly or privately, a bigger excerpt of your CSS so I can try to replicate this bug locally?

@ngyikp
Copy link
Contributor

ngyikp commented May 10, 2017

Coincidentally, I came upon this same exception, and it turns out to be a problem with -webkit- prefix handling:

.abc {
  -webkit-animation: something 0.5s 1 ease-in-out;
  -webkit-animation-fill-mode: both;
}

@jakubpawlowicz
Copy link
Collaborator

@ngyikp looks like it! 👍

@jakubpawlowicz jakubpawlowicz added this to the 4.1.2 milestone May 10, 2017
@mikestreety
Copy link
Author

mikestreety commented May 10, 2017 via email

jakubpawlowicz added a commit that referenced this issue May 10, 2017
Why:

* Components of vendor prefixed `animation` were incorrectly derived
  from an unprefixed property leading to errors.
@mikestreety
Copy link
Author

Thanks @jakubpawlowicz

jakubpawlowicz added a commit that referenced this issue May 10, 2017
Why:

* Components of vendor prefixed `animation` were incorrectly derived
  from an unprefixed property leading to errors.
@jakubpawlowicz
Copy link
Collaborator

The fix is out in 4.1.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants