Improve `@at-root` behavior #147

maranomynet · 2022-09-29T16:57:23Z

This is a work in progress PR.

I haven't looked into index.js yet, so I don't really know my chances of fixing any of these myself.

Feel free to either clone/take over this patch branch, or push commits to it.

maranomynet · 2022-09-29T16:58:08Z

I'm looking towards https://www.sassmeister.com/ for the expected behavior in each of these test cases

maranomynet · 2022-09-30T10:58:07Z

Note to self:
In 52a602c I add some basic type definitions (via JSDoc). This immediately surfaces some obvious problems.

ai · 2022-09-30T11:24:03Z

Ping me, when implementation will be ready. It looks very interesting.

maranomynet · 2022-09-30T14:03:36Z

Making small progress... 2/7 failing tests now passing.

maranomynet · 2022-10-05T07:41:11Z

Hi @ai I'm done now.

index.d.ts

index.js

As per examples given in the postcss docs: https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md#step-4-change-nodes

maranomynet · 2022-10-05T10:43:38Z

Right now, I can't think of any more cases to test, and everything passes.

Feel free to add more tests if you can think of any.

maranomynet · 2022-10-05T10:52:03Z

I've basically moved most of the at-root handling out of the Rule method and into two separate tree-walk passes — first on Once where I normalize and prepare the at-root nodes, and then on RootExit where I unwrap and drill upp the tree and split up blocks as needed.

What's left in the Rule method is just the conditional selector merging faff.

postcss/postcss-nested#147

ai · 2022-10-05T19:45:13Z

index.js

+
+    RootExit(root) {
+      if (root[hasRootRule]) {
+        root.walk(node => {


walkAtRules() will be readable and could be even a little faster

ai · 2022-10-05T19:45:40Z

index.js

-    Rule (rule, { Rule }) {
+
+    Once(root) {
+      root.walk(node => {


Use walkAtRules() here as well

ai · 2022-10-05T23:26:01Z

Looks good. I will try to merge it tomorrow.

Note that I am planning to remove type comments since I do not use them.

maranomynet · 2022-10-05T23:34:13Z

Type comments lower the barrier of entry for others to make PRs.
Also, in this case adding type hints at the start immediately surfaced errors that were hiding in plain sight.

ai · 2022-10-05T23:38:04Z

Unofficial nested has no future. We will not have too many PRs in the future.

Not sure how/why this didn't show up yesterday or the day before. VSCode's ESLint plugin must have silently crashed or something.

ai · 2022-10-06T17:17:32Z

Thanks. Released in 6.0.

maranomynet force-pushed the patch-1 branch 2 times, most recently from c6f2c2c to 52a602c Compare September 30, 2022 10:52

maranomynet force-pushed the patch-1 branch 2 times, most recently from 4351e48 to 2beaff0 Compare September 30, 2022 14:03

maranomynet force-pushed the patch-1 branch 4 times, most recently from dc918cc to 084f23e Compare September 30, 2022 14:45

maranomynet added 13 commits October 1, 2022 12:47

tests: Normalize expected and actual strings to reduce noise

34ead38

style: Add @ts-check and JSDoc type definitions to surface errors

87d30eb

tests: Add more passing tests

f848d45

tests: Add failing tests for valid uses of @at-root

0832195

style: Rename functions and variables for clarity

8004a96

rafactor: Resolve non-critical/false-positive type-checking issues

8d55747

fix: Logic/typing error when handling @at-root rules

c7a73d6

refactor: Simplify existing logic

800d131

fix: Move all preceeding comments with rule

e427a6d

fix: @layer blocks should also bubble

16c7282

fix: Correctly handle with/without parameters on @at-root

37c75f9

feat: Add option rootRuleName to rename the custom @at-root rule

3561b2e

style: Auto formatting/linting

59d784e

maranomynet force-pushed the patch-1 branch from 084f23e to 59d784e Compare October 5, 2022 07:36

maranomynet commented Oct 5, 2022

View reviewed changes

index.d.ts Show resolved Hide resolved

maranomynet commented Oct 5, 2022

View reviewed changes

index.d.ts Outdated Show resolved Hide resolved

maranomynet commented Oct 5, 2022

View reviewed changes

index.js Outdated Show resolved Hide resolved

fix: Remove hasRootRule optimization

d085b71

maranomynet force-pushed the patch-1 branch from 810bc2d to d085b71 Compare October 5, 2022 08:14

fix: Add back hasRootRule optimization, scoped to root node

99823d2

As per examples given in the postcss docs: https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md#step-4-change-nodes

maranomynet force-pushed the patch-1 branch from 67bf4f4 to 99823d2 Compare October 5, 2022 08:27

maranomynet added 2 commits October 5, 2022 08:35

docs: Update README

de5fbfc

style: Reformat test input

46829dc

maranomynet force-pushed the patch-1 branch 3 times, most recently from 5a8dd55 to cf2bbf0 Compare October 5, 2022 09:25

maranomynet added 2 commits October 5, 2022 10:38

tests: Add more @at-rule test, some failing

57655e3

fix: Failing @at-root edge cases

0b16af0

maranomynet force-pushed the patch-1 branch from cf2bbf0 to 0b16af0 Compare October 5, 2022 10:39

style: Fix typo in function name, remove console.log

4789f12

maranomynet added a commit to rvk-utd/hanna that referenced this pull request Oct 5, 2022

chore: Patch postcss-nested while PR is merged

63dc2eb

postcss/postcss-nested#147

maranomynet added a commit to rvk-utd/hanna that referenced this pull request Oct 5, 2022

chore: Patch postcss-nested while PR is merged

0af4c61

postcss/postcss-nested#147

maranomynet added a commit to rvk-utd/hanna that referenced this pull request Oct 5, 2022

chore: Patch postcss-nested while PR is merged

1d3fa79

postcss/postcss-nested#147

refactor: Remove @ts-check hack code path, use type assertion instead

cb127a3

ai reviewed Oct 5, 2022

View reviewed changes

maranomynet added 2 commits October 5, 2022 23:07

test: Remove comments

738c417

refactor: Use root#walkAtRules

91448d3

chore: Tweak eslint rule value

fcbb368

refactor: Fix linting errors …

ce88aac

Not sure how/why this didn't show up yesterday or the day before. VSCode's ESLint plugin must have silently crashed or something.

ai merged commit 62a5b85 into postcss:main Oct 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve `@at-root` behavior #147

Improve `@at-root` behavior #147

maranomynet commented Sep 29, 2022

maranomynet commented Sep 29, 2022

maranomynet commented Sep 30, 2022 •

edited

ai commented Sep 30, 2022

maranomynet commented Sep 30, 2022 •

edited

maranomynet commented Oct 5, 2022

maranomynet commented Oct 5, 2022

maranomynet commented Oct 5, 2022 •

edited

ai Oct 5, 2022

ai Oct 5, 2022

ai commented Oct 5, 2022

maranomynet commented Oct 5, 2022

ai commented Oct 5, 2022

ai commented Oct 6, 2022

Improve @at-root behavior #147

Improve @at-root behavior #147

Conversation

maranomynet commented Sep 29, 2022

maranomynet commented Sep 29, 2022

maranomynet commented Sep 30, 2022 • edited

ai commented Sep 30, 2022

maranomynet commented Sep 30, 2022 • edited

maranomynet commented Oct 5, 2022

maranomynet commented Oct 5, 2022

maranomynet commented Oct 5, 2022 • edited

ai Oct 5, 2022

Choose a reason for hiding this comment

ai Oct 5, 2022

Choose a reason for hiding this comment

ai commented Oct 5, 2022

maranomynet commented Oct 5, 2022

ai commented Oct 5, 2022

ai commented Oct 6, 2022

Improve `@at-root` behavior #147

Improve `@at-root` behavior #147

maranomynet commented Sep 30, 2022 •

edited

maranomynet commented Sep 30, 2022 •

edited

maranomynet commented Oct 5, 2022 •

edited