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

Silently breaks on @mixin and @function blocks which has 3 or more parameters with default value #12

Closed
bencergazda opened this issue Jul 24, 2017 · 6 comments

Comments

@bencergazda
Copy link
Contributor

bencergazda commented Jul 24, 2017

Try to @import a file, which contains a @function or @mixin which has more than three parameters with a default value:

@function test($attr1, $attr2, $attr3: null, $attr4: null, $attr5: null) {
	@return 'hello';
}

The extractor will halt and you will have to terminate the batch job manually.

Strange, but it doesn't halt on eg. @function test($attr1, $attr2, $attr3, $attr4, $attr5), so it's not the @function˛ or @mixin block itself, which causes the error. It's also OK with @function test($attr1, $attr2, $attr3, $attr4: null, $attr5: null) {, so when there's only two (or less) attributes with default a value.

@jgranstrom
Copy link
Owner

@bencergazda I am pretty sure this is similar to #2 and related to the regex parsing not exhausting and getting stuck. I am currently reworking this to use the AST as basis for the source parsing which will clear out similar issues with parsing. I will look into patching this specific issue as an intermediate step asap.

@adamgruber
Copy link

adamgruber commented Aug 26, 2017

I ran into the same thing. It definitely seems to be related to the default value. If I have a mixin/function that specifies a default value, followed by a mixin/function, the parser freezes. If it is followed by a normal declaration, it will work. Additionally it works fine when the mixin/function is the last declaration in the file.

// Fails
@mixin fg($color: green) {
  color: $color;
}
@function get-color() {
  @return red;
}
// Works
@function get-color() {
  @return red;
}
@mixin fg($color: green) {
  color: $color;
}
// Works
@mixin fg($color: green) {
  color: $color;
}
$other: blue;
@function get-color() {
  @return red;
}

I've tried looking into the regex but I'm not great with them and haven't yet figured out how to tweak it for this case.

Reading through other issues I see that you're working on switching from regex to using the AST. How's progress on that? Any chance you have time to look into patching this issue in the meantime?

@jgranstrom
Copy link
Owner

@adamgruber thank you for additional details. The regex becomes pretty complex and is prone to issues like this since source files can have so many different variations. I am hoping to have a version that uses AST in the next week or 2, though I am a bit short on time at the moment.

I will spend a couple of hours probably tomorrow and see if there's a quick regex fix until then.

@jgranstrom
Copy link
Owner

@adamgruber after some trials I feel that a regex quick fix becomes a bit of a whack-a-mole exercise and is not worth spending too much time on at this point. I will make sure that we have an AST solution in place soon instead that will fix it on a more fundamental level. I will update when there's a new release using AST parsing.

@adamgruber
Copy link

@jgranstrom Definitely makes sense to focus on an AST solution. I appreciate you taking time to look into this. I've got a project that uses this library. (https://github.com/adamgruber/styled-sass-theme) For now I'll just add a note about this use case and how to work around it. Looking forward to the next release.

@jgranstrom
Copy link
Owner

@adamgruber @bencergazda there is a new release 1.0.0 as of today that is using AST instead of regex to parse the source files. This should get rid of all of these parsing errors. Try it out and see if you still experience any issues.

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

No branches or pull requests

3 participants