-
Notifications
You must be signed in to change notification settings - Fork 355
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
Add support for loading modules and using members from them #603
Conversation
The value of an `AtRule` can be null, so it should not be visited in that case. Ran across this issue when I attempted to run the module migrator on a stylesheet containing `@font-face` (which has children, but no value).
You only need to review the commits after the merge; everything else was already reviewed when it landed on |
/// level as `.sass` and `.scss`, while `@import` prefers `.sass` and `.scss` | ||
/// over `.css`. It's admittedly hacky to set this globally, but `@import` will | ||
/// eventually be removed, at which point we can delete this and have one | ||
/// consistent behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thought of a question about this new priority for .css
files. Does this cause a problem for users that output .css
to their source tree next to the original Sass file? For example, will the first build work because it finds foo.scss
but then on the second build throws an error because it finds foo.scss
and foo.css
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question. LibSass, which incorrectly supports .css
files right now, does produce a conflict error here, but that doesn't necessarily mean it's the best option. Let's talk this through in person on Monday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok with me if you want to commit these changes and revise how you see fit per language/68.
lib/src/parse/stylesheet.dart
Outdated
/// | ||
/// These are collected at parse time because they affect the variables | ||
/// exposed by the module generated for this stylesheet, *even if they aren't | ||
/// evaluated*. This allows us to ensure that the kstylesheet always exposes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "kstylesheet"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
See sass/sass-spec#1349