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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

dartsass: Import CSS without extension at compile time #10593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions resources/resource_transformers/tocss/dartsass/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,22 +68,24 @@ $moolor: #fff;
moo {
color: $moolor;
}
-- assets/scss/another.css --

-- node_modules/foo/another.css --
another {
color: #000;
}
-- assets/scss/main.scss --
@import "moo";
@import "regular.css";
@import "moo";
@import "another.css";
@import "another";

/* 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 •
edited
Loading

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).

{{ $r := resources.Get "scss/main.scss" | toCSS $cssOpts }}
T1: {{ $r.Content | safeHTML }}

`

b := hugolib.NewIntegrationTestBuilder(
Expand All @@ -97,15 +99,18 @@ T1: {{ $r.Content | safeHTML }}
// Dart Sass does not follow regular CSS import, but they
// get pulled to the top.
b.AssertFileContent("public/index.html", `T1: @import "regular.css";
@import "another.css";
moo {
color: #fff;
}

moo {
color: #fff;
}


another {
color: #000;
}

/* foo */`)
}

Expand Down Expand Up @@ -291,7 +296,7 @@ body {
body {
background: url(vars.$image) no-repeat center/cover;
font-family: vars.$font;
}
}
}

p {
Expand Down Expand Up @@ -341,7 +346,7 @@ image = "images/hero.jpg"
body {
body {
background: url(vars.$image) no-repeat center/cover;
}
}
}

p {
Expand Down
4 changes: 2 additions & 2 deletions resources/resource_transformers/tocss/dartsass/transform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ func (t importResolver) CanonicalizeURL(url string) (string, error) {
if strings.Contains(name, ".") {
namePatterns = []string{"_%s", "%s"}
} else if strings.HasPrefix(name, "_") {
namePatterns = []string{"_%s.scss", "_%s.sass"}
namePatterns = []string{"_%s.scss", "_%s.sass", "_%s.css"}
} else {
namePatterns = []string{"_%s.scss", "%s.scss", "_%s.sass", "%s.sass"}
namePatterns = []string{"_%s.scss", "%s.scss", "_%s.sass", "%s.sass", "_%s.css", "%s.css"}
}

name = strings.TrimPrefix(name, "_")
Expand Down