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

Handle root-level ampersands gracefully #153

Closed
wants to merge 2 commits into from

Conversation

maranomynet
Copy link
Contributor

@maranomynet maranomynet commented Mar 31, 2023

I ran into this inconsitent behavior in the plugin.

After thinking about this for a while, I feel that postcss-nested ought to handle this gracefully and treat a root-level & selector token the same way it handles the empty selector — i.e. remove the curly block and replace all instances of & with the empty string.

(For comparison SASSMeister just errors on both of these cases, so... 🤷)

I ran into this in an actual project where we had mixin/function that got (quite reasonably) applied at the root level in certain cases, and the results felt surprisingly unelegant.

I've added these tests, but I'd be thankful if someone could provide/suggest the required changes in index.js.

@maranomynet maranomynet changed the title Root level ampersand Handle root-level ampersands gracefully Mar 31, 2023
@ai
Copy link
Member

ai commented Mar 31, 2023

How does CSS Nested spec & on the root or selector-less {}?

@maranomynet
Copy link
Contributor Author

I can't see that it touches on such cases at all.

Then again, this plugin is aiming towards SASS-style nesting, which has very different handling of & in all sorts of cases.

@maranomynet
Copy link
Contributor Author

Gracefully collapsing top-level & selectors seems like the sensible, unremarkable thing to do.

@ai
Copy link
Member

ai commented Mar 31, 2023

I prefer throwing an error.

Does Sass throw an error on both & {} and {} top levels?

@maranomynet
Copy link
Contributor Author

It does.

@ai
Copy link
Member

ai commented Mar 31, 2023

Can I ask you to add error throwing to this PR? Sorry, I am busy on another open source project.

@maranomynet
Copy link
Contributor Author

If this plugin is going to throw on both of these cases I must scramble to refactor some project code before things start breaking.

Let's just close this PR, since all it contains are tests that assert a different (non-throwing) behavior.

@maranomynet maranomynet deleted the root-level-ampersand branch March 31, 2023 14:53
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