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

Update cssesc to 3.0.0 #169

Merged
merged 1 commit into from
Feb 5, 2019
Merged

Update cssesc to 3.0.0 #169

merged 1 commit into from
Feb 5, 2019

Conversation

adamwathan
Copy link
Contributor

Closes #168

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.394% when pulling 59410c7 on adamwathan:update-cssesc into b47e255 on postcss:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.394% when pulling 59410c7 on adamwathan:update-cssesc into b47e255 on postcss:master.

t.deepEqual(attr.attribute, 'ng:foo');
t.deepEqual(tree.toString(), '[ng\\3A foo]');
t.deepEqual(tree.toString(), '[ng\\:foo]');
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm, looks something invalid (title assign attribute name requiring escape), but now no escaping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still escaped 👍 it's taking ng:foo and converting it to ng\:foo. Previously it was converting that to ng\3A foo but the : character only needs to be escaped that way in IE < 8. In IE9+ (and every other browser), it's safe to escape : as \:.

The main motivation for this change is to workaround the bug in css-loader (webpack-contrib/css-loader#578) where the leading slash is removed when it is followed by a hex character. This change is an attempt to make it possible to use as many non-standard characters as possible by not unnecessarily escaping : as \3A (which gets converted to 3A by css-loader) and instead escaping it as \: which css-loader leaves intact 👍

@alexander-akait
Copy link
Collaborator

@jonathantneal should we release this as patch?

@jonathantneal
Copy link
Member

I like this change. It requires a new major release because the maintainer of cssesc released it as a major release. I still think we should bring it in and look for any other major updates we can also take in.

Even if there are no other major updates, because this update is straight-forward, I’m also fine to release this new version as-is.

@alexander-akait
Copy link
Collaborator

@jonathantneal hm, let's do it, can you take care?

@jonathantneal
Copy link
Member

Yes. Thank you, @evilebottnawi for managing the issue and this PR.

@jonathantneal jonathantneal merged commit b1da0fa into postcss:master Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants