Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(lib): correctly handle arrays in felaSanitizeCssPlugin#573

Merged
miroslavstastny merged 4 commits intomasterfrom
fix/fela-sanitize-css-arrays
Dec 7, 2018
Merged

fix(lib): correctly handle arrays in felaSanitizeCssPlugin#573
miroslavstastny merged 4 commits intomasterfrom
fix/fela-sanitize-css-arrays

Conversation

@miroslavstastny
Copy link
Member

Fixes #572

color: ['red', 'blue'] is a valid style but felaSanitizeCssPlugin converted this to following object:

color: {
  0: 'red',
  1: 'blue'
}

which is incorrect. Now the plugin handles arrays correctly.

const processedStyles = Array.isArray(styles) ? [] : {}

Object.keys(styles).forEach(cssPropertyName => {
const cssPropertyValue = styles[cssPropertyName]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: cssPropertyName -> cssPropertyNameOrIndex? Then intend would be easier to capture a week after :)

assertCssPropertyValue(`url('../../lib')`, true)
})

describe('should pass arrays', () => {
Copy link
Contributor

@kuzhelov kuzhelov Dec 6, 2018

Choose a reason for hiding this comment

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

A bit confused with the test name, as it doesn't provide some necessary insights to me. Specifically, here:should pass array - pass untouched? Process somehow? If process, then what is the logic that will be applied?

Would expect tests to cover the following logic:

if array is passed, then for each item of an array CSS styles sanitization should be applied

@kuzhelov kuzhelov added the needs author feedback Author's opinion is asked label Dec 6, 2018
@miroslavstastny miroslavstastny added 🚀 ready for review and removed needs author feedback Author's opinion is asked labels Dec 7, 2018
@miroslavstastny miroslavstastny merged commit 2411cad into master Dec 7, 2018
@miroslavstastny miroslavstastny deleted the fix/fela-sanitize-css-arrays branch December 7, 2018 12:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants