Skip to content

test: escaped in selector#973

Merged
alexander-akait merged 1 commit intomasterfrom
issue-966
Jul 18, 2019
Merged

test: escaped in selector#973
alexander-akait merged 1 commit intomasterfrom
issue-966

Conversation

@alexander-akait
Copy link
Copy Markdown
Member

@alexander-akait alexander-akait commented Jul 17, 2019

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes #966

Breaking Changes

No

Additional Info

/cc @jquense looks all fine, can't see problems or bugs

@alexander-akait alexander-akait force-pushed the issue-966 branch 2 times, most recently from 90c8446 to 16256a3 Compare July 17, 2019 20:33
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 17, 2019 •
edited
Loading

Codecov Report

Merging #973 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #973   +/-   ##
=======================================
  Coverage   98.34%   98.34%           
=======================================
  Files          10       10           
  Lines         424      424           
  Branches      126      126           
=======================================
  Hits          417      417           
  Misses          7        7

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f51859b...1afde1d. Read the comment docs.

@alexander-akait
Copy link
Copy Markdown
Member Author

alexander-akait commented Jul 18, 2019

/cc @jquense i am merging PR, maybe problem in postcss plugin or something other in configuration? Anyway feel free to feedback in issue

@alexander-akait alexander-akait merged commit 5d2c52e into master Jul 18, 2019
@alexander-akait alexander-akait deleted the issue-966 branch July 18, 2019 10:23
jquense
jquense reviewed Jul 18, 2019
View reviewed changes
test/__snapshots__/modules-option.test.js.snap
Array [
Array [
1,
".button.hey {
Copy link
Copy Markdown
Contributor

@jquense jquense Jul 18, 2019

Choose a reason for hiding this comment

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

the bug is right here: .button.hey is very different than: .button\.hey. The former is a selector for when both .button and .hey are on an element

@jquense
Copy link
Copy Markdown
Contributor

jquense commented Jul 18, 2019

The test case here is not accurate. It illustrates the bug but doesn't contain an assert checking for it

@alexander-akait
Copy link
Copy Markdown
Member Author

alexander-akait commented Jul 18, 2019

@jquense what you expected?

@alexander-akait
Copy link
Copy Markdown
Member Author

alexander-akait commented Jul 18, 2019

Let's move discussion in #966

jquense
jquense reviewed Jul 18, 2019
View reviewed changes
test/__snapshots__/modules-option.test.js.snap
exports.push([module.id, \\".button.hey {\\\\n color: red;\\\\n}\\\\n\\", \\"\\"]);
// Exports
exports.locals = {
\\"button\\": \\"button.hey\\"
Copy link
Copy Markdown
Contributor

@jquense jquense Jul 18, 2019

Choose a reason for hiding this comment

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

This is also wrong, but if you escaped in getLocalIdent it would be fixed

This was referenced Jul 18, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jquense jquense jquense left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

values in selectors aren't escaped properly.

2 participants

@alexander-akait @jquense