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

dartsass: Import CSS without extension at compile time #10593

Closed

Conversation

jmooring
Copy link
Member

@jmooring jmooring commented Jan 1, 2023

Applicable to Dart Sass only:

  • Sass imports with the .css extension indicate a plain CSS @import.
  • Sass imports without the .css extension are imported at compile time.

Fixes #10592

Applicable to Dart Sass only:

- Sass imports with the .css extension indicate a plain CSS @import.
- Sass imports without the .css extension are imported at compile time.

Fixes gohugoio#10592

/* foo */
-- assets/scss/regular.css --

-- config.toml --
-- layouts/index.html --
{{ $r := resources.Get "scss/main.scss" | toCSS (dict "transpiler" "dartsass") }}
{{ $cssOpts := (dict "includePaths" (slice "node_modules/foo") "transpiler" "dartsass") }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original issue, as I understand it, was about importing CSS files from /assets; with that I don't understand why you move the CSS to node_modules?

Copy link
Member Author

@jmooring jmooring Jan 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original issue, as I understand it, was about importing CSS files from /assets

The OP incorrectly commented on what was working vs. what was not. I discovered their error when I began testing this, but did not ask the OP to verify their comments until yesterday. They confirmed that the original comments were reversed, but the issue title is correct.

We have no problem importing a CSS file in the assets directory at compile time. But when you import a CSS file from a mounted directory (either through module.mounts or the includePaths slice), the result is a plain import.

I would be happy to clarify the test if necessary, but I was trying to minimize the diff for you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be happy to clarify the test if necessary, but I was trying to minimize the diff for you.

To minimise the diff would be dropping the includePaths change, which is what confuses me -- even after rereading the original issue. I think I understand the issue, and if I understand it correctly that should be possible to reproduce by just importing a CSS file without using the extension when the file is imported into /assets in a mount (which lives outside of what includePaths is all about).

@jmooring
Copy link
Member Author

bep, I apologize for the delay on this. I have been having intermittent failures when testing. It turns out the tests were intermittently skipped due to:

if !dartsass.Supports() {
	t.Skip()
}

That is not your problem.

I had been running the tests (a) within VS Code, and (b) with dart-sass-embedded installed via Brew. There are no problems when dart-sass-embedded is installed in /user/local/bin. I still haven't worked out the correct settings in VS Code to address this, so I'll stick with the install in /user/local/bin.

@bep
Copy link
Member

bep commented Feb 23, 2023

Bump ... Is this still an issue/relevant?

@jmooring
Copy link
Member Author

Yes, still relevant, got distracted. I can look at it again later today (much later today).

@jmooring jmooring closed this Feb 23, 2023
@jmooring jmooring deleted the fix/sass-import-css-without-ext branch February 23, 2023 22:29
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to import mounted CSS into SCSS.
2 participants