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

refactor: v3.0 #113

Merged
merged 26 commits into from Sep 21, 2018
Merged

refactor: v3.0 #113

merged 26 commits into from Sep 21, 2018

Conversation

shellscape
Copy link
Owner

@shellscape shellscape commented Sep 17, 2018

Codebase refactor and update for version 3.0.0.

Tasks:

Breaking Changes

  • @import are now AtRule with import: true property
  • LESS variable nodes are now AtRule with variable: true property
  • LESS mixin nodes are now Rule or AtRule (depending on where they are used/declared) with mixin: true property
  • LESS variables cannot contain whitespace between the variable name and the colon (e.g. @thing : value) and will be parsed as regular CSS.
  • !important no longer parsed separately if a space separates the ! and important.

@shellscape
Copy link
Owner Author

/cc @jwilsson

@shellscape shellscape mentioned this pull request Sep 17, 2018
6 tasks
@shellscape
Copy link
Owner Author

@ai leveraging PostCSS@7, I was able to do away with a large amount of code. lib/LessParser.js is where most of the magic happens for some LESS-specific needs. I'm having trouble getting the base parser to recognize interpolated code, however. e.g. @{selector}:hover { @{prop}-size: @{color} }. If you have any ideas on how to accomplish that in the same pattern as what I've committed in aa8790f, I would appreciate it.

@ai
Copy link

ai commented Sep 18, 2018

I'm having trouble getting the base parser to recognize interpolated code, however. e.g. @{selector}:hover { @{prop}-size: @{color} }

Is “base parser” postcss/lib/parser?

What exactly problem do you have?

@codecov-io

This comment has been minimized.

@codecov-io

This comment has been minimized.

1 similar comment
@codecov-io

This comment has been minimized.

@shellscape
Copy link
Owner Author

@ai

Is “base parser” postcss/lib/parser?

Yes. lib/LessParser extends postcss/lib/parser.

What exactly problem do you have?

test/parser/_interpolation.js contains examples of LESS variable interpolation. PostCSS doesn't support this sort of syntax by default (and I wouldn't expect it to). I have been using a method of overriding select methods to re-use PostCSS node types. For example, using AtRule as the node type and extending it with properties such as variable: true and mixin: true. I have done the same with Comment to allow for inline comments that are supported in LESS.

I'm attempting to use the same approach to allow parsing of interpolation identifiers. For example:

@{selector}:hover { ... }

would be parsed as a Rule with a selector value of '@{selector}:hover', just as foo:hover { ... } would be parsed as a Rule with a selector value of foo:hover. The same would go for Declaration nodes to allow the prop value to contain interpolation identifiers.


So far I haven't had much luck in getting things to play nicely and allow that. I tried an approach overriding unknownWord but had to abandon it to complete the other features. The rewrite is complete, except for Interpolation support. I will start work on that again tomorrow but wanted to ask if you could see a solution.

@codecov-io

This comment has been minimized.

@ai
Copy link

ai commented Sep 18, 2018

I suggest you to create special token for @{ symbols. Then you can make special logic with it in other.

Or you can keep current tokens, but override parse method.

@codecov-io

This comment has been minimized.

@shellscape
Copy link
Owner Author

@ai interesting idea. could you share how you see that looking?

@codecov-io

This comment has been minimized.

@ai
Copy link

ai commented Sep 18, 2018

Sorry, I will not have time for it in next few weeks, because I am preparing for conference talk.

Ask a questions.

@codecov-io

This comment has been minimized.

@jwilsson
Copy link
Collaborator

@shellscape Wow, this looks amazing!

You seem to have a pretty good streak going, but I'll be happy to help out with testing across downstream projects.

@shellscape
Copy link
Owner Author

This PR is ready for test use. @jwilsson

@jwilsson
Copy link
Collaborator

@shellscape Great! I'll take a look over the weekend.

@shellscape shellscape merged commit a28a329 into master Sep 21, 2018
@shellscape shellscape deleted the refactor/3.0 branch September 26, 2018 16:49
@thorn0
Copy link

thorn0 commented Nov 18, 2019

LESS variables cannot contain whitespace between the variable name and the colon (e.g. @thing : value) and will be parsed as regular CSS.

What is the motivation behind this? Less allows whitespace in that position.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment