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

[dart-sass]: hoistUseStatements causes problems with @use ... with() #135

Closed
tomas-hartman opened this issue Jan 6, 2021 · 7 comments · Fixed by #140
Closed

[dart-sass]: hoistUseStatements causes problems with @use ... with() #135

tomas-hartman opened this issue Jan 6, 2021 · 7 comments · Fixed by #140

Comments

@tomas-hartman
Copy link

Hi, I encountered a problem when using the hoistUseStatements option when I need to use imported variables inside with statement. Compilation ends up with SassError: There is no module with the namespace "colors". error.

I suppose this is because resources are @imported, so they are not available at the time of compilation of @uses. (Of course it also fails when not using hoistUseStatements, because SassError: @use rules must be written before any other rules.).

I'd suggest adding some "importWithUse" switch or "usedResources", to allow the import to be at the very beginning of the file. This might also require adding some options for @forward later.

Example

// webpack loaders settings

{
  test: /\.scss$/,
  loader: 'sass-resources-loader',
  options: {
    resources: "_colors.scss",
    hoistUseStatements: true
  }
}
// _colors.scss (simplified)

$dark-blue: blue;
$dark-orange: orange;
$black: black;
// webpack using sass-resources-loader

// beginning of file
@use 'bootstrap-base' with (
  $dark: colors.$dark-blue,
  $primary: colors.$dark-orange,
  $secondary: colors.$black,
);

other-selectors-mixins-etc {
 // ...
}

// -> SassError: There is no module with the namespace "colors".
@justin808
Copy link
Member

justin808 commented Feb 18, 2021

@Tomburgs Any thoughts on this?

@tomas-hartman could you submit a PR with what you're thinking?

Is this only with dart-sass and not node-sass?

@Tomburgs
Copy link
Member

@justin808 I actually was working on improving that.

The problem we're having is that we're not using actual AST parser, as all we actually need to do is find the last occurrence of @use statement, thus doing it with regex is easier.

Currently the regex we're using expects everything to be on the same line, we could fix it by doing something like this, which would allow there to be line breaks between parenthesis:

// Matches opening and closing parenthesis across multiple lines
const multilineParenthesisRegex = /\([\s\S]+);/;
// Finds any @use statement
const useRegex = `^@use .*(?:${multilineParenthesisRegex})?\n?$`;
// Same as above, but adds the m (multiline) flag
const useRegexTest = new RegExp(useRegex, 'm');
// Makes sure that only the last instance of `useRegex` variable is found
const useRegexReplace = new RegExp(`${useRegex}(?![\\s\\S]*${useRegex})`, 'gm');

I could try to make a PR for this later the day, but I'm also curious if anyone has another approach in mind?

@justin808
Copy link
Member

@Tomburgs any update on this one?

@Tomburgs
Copy link
Member

@justin808 @tomas-hartman I created PR #140 to fix this.
Sticking with the regex approach for now.

@justin808
Copy link
Member

@tomas-hartman, @lucpotage, can one of you give me 👍 on if we're ready for a release with #140 ? It's merged to master!

Big thanks @Tomburgs for this change!!!!

@justin808
Copy link
Member

OK --I shipped. 🤞

@mrleblanc101
Copy link

Does not seem to work if using :export somewhere. #149

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

Successfully merging a pull request may close this issue.

4 participants