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

remove() not removing since 8.1.14 #1526

Open
rigor789 opened this issue Feb 11, 2021 · 4 comments
Open

remove() not removing since 8.1.14 #1526

rigor789 opened this issue Feb 11, 2021 · 4 comments

Comments

@rigor789
Copy link

After version 8.1.14 calling rule.remove() or decl.remove() doesn't remove nodes correctly.

The only change between the working/non-working version is a187464 however I stepped through with the debugger, and couldn't see anything different in the calls, in fact logging out the fromOffset return values and diffing the outputs of the working/non-working version, they are identical, leaving me very puzzled.

Tried reducing the variables in an empty project, but seems to be working fine there.

The behavior can be seen in https://github.com/rigor789/nativescript-tailwind on the feat/tailwind2 branch. Steps to reproduce:

  1. git clone https://github.com/rigor789/nativescript-tailwind.git
  2. git checkout feat/tailwind2
  3. yarn && yarn build
  4. Expect no changes in dist/tailwind.css (git diff)
  5. change "postcss": "8.1.13" to 8.1.14 (any higher version produces same results)
  6. yarn && yarn build again
  7. dist/tailwind.css now has many empty rules (seen in git diff)

We are calling remove on rules that are empty here: https://github.com/rigor789/nativescript-tailwind/blob/bfb0c2bb90b4a72c569b81a3096a5262bc8f2a30/src/removeUnsupported.js#L49-L51

We are also calling remove on declarations here: https://github.com/rigor789/nativescript-tailwind/blob/bfb0c2bb90b4a72c569b81a3096a5262bc8f2a30/src/removeUnsupported.js#L138-L142

I'd be happy to work on a fix, but would like to get some help figuring out what's wrong 🤗

@ai
Copy link
Member

ai commented Feb 13, 2021

Tailwind contains many different tools inside. It is hard to debug it.

Can you try to remove Tailwind tools to find a few plugins to reproduce the issue?

@ilyavf
Copy link

ilyavf commented Mar 10, 2021

@rigor789 @ai, I put some debugger logs, and compared the 2 versions on what the difference in behaviour is related to the unwanted empty rules.

On 8.1.13 the Rule for e.g. .cursor-wait selector is called twice - at first with rule.nodes.length equal to 1, and then equal to 0, and thus it gets removed under the condition of the lines you mentioned .

On 8.1.14 however the Rule for the same selector is called only once - when the rule.nodes.length is 1. Thus the rule does not get removed:

8.1.13:
>>> RULE: rule.selector = .cursor-wait
- rule.nodes.length = 1
>>> RULE: rule.selector = .cursor-wait
- rule.nodes.length = 0

8.1.14:
>>> RULE: rule.selector = .cursor-wait
- rule.nodes.length = 1

Hope this helps. I am unfamiliar with PostCSS internals but can try to dive deeper if anybody shares any thoughts on this.

@rigor789
Copy link
Author

Thanks for looking into it @ilyavf - interesting find! I will likely have some time to take a fresh look at this sometime next week, or the week after.

What's interesting is that I've tried running the empty rule removal as a separate plugin and I recall seeing the same behavior - but will need to test that again to double check.

@ai
Copy link
Member

ai commented Mar 11, 2021

Very strange. We didn’t change anything with plugin API between 8.1.13 and 8.1.14.

Here is only commit related to JS sources: a187464

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

No branches or pull requests

3 participants