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

Auto-detect sass vs sass-embedded #290

Closed
JoostKersjes opened this issue Sep 23, 2024 · 3 comments · Fixed by #292
Closed

Auto-detect sass vs sass-embedded #290

JoostKersjes opened this issue Sep 23, 2024 · 3 comments · Fixed by #292

Comments

@JoostKersjes
Copy link

I'm using "sass-true": "8.0.0" and incorrectly assumed that it would work with "sass-embedded": "1.79.2".

The reason for replacing sass with sass-embedded is mostly for improved performance.

Error message:

 FAIL  test/sass.test.js [ test/sass.test.js ]
Error: Cannot find Dart Sass (`sass`) dependency.
 ❯ Object.runSass ../node_modules/.pnpm/sass-true@8.0.0/node_modules/sass-true/src/index.ts:112:13
 ❯ test/sass.test.js:13:44
     11|   });
     12| 
     13|   sassTestFiles.forEach((file) => sassTrue.runSass({ describe, it }, file));
       |                                            ^
     14| });
     15| 
 ❯ test/sass.test.js:13:17
@JoostKersjes
Copy link
Author

The pipelines on our project did not fail for the change to replace sass with sass-embedded. I'm not exactly sure yet why they only started failing a day after the changes were merged, but that alone makes me think this is to do with caching.

@JoostKersjes
Copy link
Author

JoostKersjes commented Sep 23, 2024

Sorry for creating an issue in a rush. It turns out that this problem can be avoided by simply passing the sass-embedded as an option to True like such:

import { dirname, resolve } from "node:path";
import { fileURLToPath } from "node:url";
import * as sass from "sass-embedded";
import sassTrue from "sass-true";
import { describe, it } from "vitest";

describe("Sass", () => {
  const sassTestFiles = Object.keys(
    import.meta.glob("./**/*.test.scss", { eager: true }),
  ).map((relativePath) => {
    return resolve(dirname(fileURLToPath(import.meta.url)), relativePath);
  });

  sassTestFiles.forEach((file) =>
    sassTrue.runSass({ describe, it, sass }, file),
  );
});

I think it would still be beneficial if sass-true could auto-detect if it should grab the sass or sass-embedded package, but this option workaround relieves any urgency.

@jgerigmeyer jgerigmeyer changed the title Support sass-embedded Auto-detect sass vs sass-embedded Sep 23, 2024
@jgerigmeyer
Copy link
Member

@JoostKersjes I think that's a good idea -- I've renamed this issue.

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

Successfully merging a pull request may close this issue.

2 participants