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

Port to PostCSS 8 API. #71

Merged
merged 2 commits into from Oct 31, 2020
Merged

Port to PostCSS 8 API. #71

merged 2 commits into from Oct 31, 2020

Conversation

ludofischer
Copy link
Contributor

@ludofischer ludofischer commented Sep 24, 2020

Hi, by trying to port the plugin to use the more efficient PostCSS 8 API, I’ve noticed there’s some confusing logic inside the plugin that prevents this. In the end, as it is, this pull request does not add much except maybe future proofing, but I thought I’d share it to save you some time if you ever try porting yourself.
If I replace the walkDecls() call with Declaration(), the plugin goes into an infinite loop when I run the tests. The PostCSS migration guide says

plugins will re-visit all changed or added nodes. You should check if your transformations were already applied and if that is the case—ignore those nodes. Only Root and RootExit listeners will be called exactly once.

I believe bug4() sees the same node again after bug6() processes it and adds the 0% flex basis value that bug6() then removes again and so on. Because I am not sure I understand the intention adding then removing the 0%, I went back to walking the tree with the Root listener.

@ludofischer ludofischer force-pushed the postcss8 branch 2 times, most recently from 5c621de to 1e21990 Compare Sep 24, 2020
@luisrudge luisrudge merged commit ff3e5a2 into luisrudge:master Oct 31, 2020
@luisrudge
Copy link
Owner

luisrudge commented Oct 31, 2020

Thanks for the PR! Released postcss-flexbugs-fixes@5.0.0

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.

None yet

2 participants