feat: Support SASS #52

ntt261298 · 2022-04-16T16:58:10Z

Features

Support SASS
- Transform SASS file to CSS content, then create <style> tag with CSS content and append to document.
- Support configured SASS files by transforming SASS files to CSS files, saving CSS files to cache and serving all CSS files by our preview server.

Todo:

Support custom loadPaths: Currently, jest-preview only supports relative scss import via @import, @use. In real projects, we can add custom loadPaths to let sass know where to find the imported files.
Support ~ import: Example: @use '~nprogress/nprogress'. This is a feature of sass-loader, or we can add additional config to achieve this. Example in Vite:

  resolve: {
     alias: [
       {
         find: /^~.+/,
         replacement: (val) => {
           return val.replace(/^~/, "");
         },
       },
     ],
   },

@use

- Todo: Support @use, @import for SASS

nvh95 · 2022-04-16T17:38:38Z

examples/vite-react/config/jest/setupTestsBeforeEnv.js

+jestPreviewConfigure({
+  externalCss: ['src/index.css', 'src/global-style.scss'],
+  sassLoadPaths: ['src'], // Root for @use, @import
+});


@ntt261298 Is there any reason we need to move jestPreviewConfigure from setupFilesAfterEnv to setupFiles?

I thought that setupFilesAfterEnv will be run before the transformers, but actually both setupFilesAfterEnv and setupFiles are run before transformers, so I reverted to use setupFilesAfterEnv

nvh95 · 2022-04-16T17:40:44Z

examples/vite-react/src/assets/css/style.scss

@@ -0,0 +1,7 @@
+@import './partial-style';


I see the file is _partial-style.scss (more _). Is this Saas feature or a typo?

It's the feature of Sass,_ indicates that is a partial sass, so sass will not compile it to a CSS unless we use that in another sass file.

Also: .sass and .scss solve the same problem, but the syntax is a bit different, .scss has the syntax more similar to .css (contain semicolons, brackets) while .sass doesn't: https://www.geeksforgeeks.org/what-is-the-difference-between-scss-and-sass/#:~:text=SASS%20(Syntactically%20Awesome%20Style%20Sheets,of%20the%20existing%20CSS%20syntax.

nvh95 · 2022-04-16T17:43:40Z

src/configure.ts

+    if (cssFile.endsWith('.scss') || cssFile.endsWith('.sass')) {
+      const sass = require('sass');
+


This is not safe since user might not have saas installed locally. Might need to try catch

Yeah, also, sass-loader supports multiple sass implementation (node-sass, dart-sass,...), maybe we can refer to that

nvh95 · 2022-04-16T17:45:26Z

src/transform.ts

+  } catch {
+    // File doesn't exist
+    sassLoadPaths = JSON.stringify([]);
+  }


Can we guarantee the error is always File doesn't exist?
Can we assert if the error is exactly that case?

I think we can move to use fs.existsSync to check of the config file exists

nvh95 · 2022-04-16T17:48:14Z

src/transform.ts

+const sassSrc = ${JSON.stringify(src)};
+
+const result = sass.compileString(sassSrc, {
+  loadPaths: ${sassLoadPaths},
+});


Actually, we can compile saas in the transformer to leverage caching feature #51 (now we haven't implemented yet, but we can prepare) instead of run time. CSS Modules (#40) might need to change also.
Do we have any other reason we need to convert saas to run time?

We don't have other reason to convert sass to run time, so I move it out of the return string

nvh95 · 2022-04-16T17:51:50Z

src/configure.ts

+      // Transform sass to css
+      const sassDestinationFile = destinationFile.replace(
+        /\.(scss|sass)$/,
+        '.css',
+      );


I think move this declaration next to fs.writeFileSync(sassDestinationFile, sassResult.css); is more readable

# Conflicts: # package-lock.json # package.json # src/configure.ts

… sass-support

… sass in transform.ts

nvh95 · 2022-04-18T18:49:16Z

src/configure.ts

-    );
+    // Example: src/common/styles.css => cache-src___common___styles.css
+    const delimiter = '___';
+    const destinationBasename = `cache-${basename.replace('/', delimiter)}`;


This will replace the first /.

Suggested change

const destinationBasename = `cache-${basename.replace('/', delimiter)}`;

const destinationBasename = `cache-${basename.replace(/\//g, delimiter)}`;

I don't think this logic correct since we are using basename

nvh95 · 2022-04-18T18:51:32Z

src/configure.ts

+
+    // If sass file is included, we need to transform it to css
+    if (cssFile.endsWith('.scss') || cssFile.endsWith('.sass')) {
+      const { exec } = require('child_process');


I think we can use import here.

nvh95 · 2022-04-18T18:54:26Z

src/transform.ts

+    sass = require('sass');
+  } catch (err) {
+    console.log(err);
+    return `module.exports = ${JSON.stringify(src)};`;


I think fileame is a better option for a fallback.

nvh95

Can you update scss code to also use @use?
Can you update scss code to use ~ (e.g:@use '~nprogress/nprogress';) (loading a css file from the nprogress node module)
Reading Create React App's scss page gives me a lot of information (@use, how they handle loadPaths via environment variable SASS_PATH, node-sass and LibSass are deprecated, SCSS Modules)
Given node-sass and LibSass are deprecated (need more source), let's consider if we need to support those processors. I think the strategy can be: release with Dart Sass, then support other Sass processors in the next releases. If we go with that strategy, remember to mention we are currently support only Dart Sass.
Can you update scss code to demo. That folder is to be used for development than examples/vite-react. I think we can add an other example sass to showcase how jest-preview works with sass (The idea is from Next.JS https://github.com/vercel/next.js/tree/canary/examples)

nvh95 · 2022-04-19T00:29:50Z

src/configure.ts

+      exec(
+        `./node_modules/.bin/sass ${cssFile} ${cssDestinationFile} --no-source-map`,
+        (err: any) => {
+          if (err) {
+            console.log(err);
+          }
+        },
+      );
+      return;
+    }


Since the reason to use exec here is special (try not to run sass in jsdom environment), Can you add a detail comment here? @ntt261298?

nvh95 · 2022-04-19T00:51:01Z

src/configure.ts

+
+      // Transform sass to css and save to cache folder
+      exec(
+        `./node_modules/.bin/sass ${cssFile} ${cssDestinationFile} --no-source-map`,


I have a little bad feeling about using ./node_modules/.bin/sass directly. I'm afraid it's not gonna work in some circumstances (e.g: monorepo? but we are not testing for monorepo just yet anyway).

I'm not sure, since this is pretty similar to the problem I have to solve when adding postinstall script.

No clear action items now. I think just to keep this implementation in mind for now.

nvh95 · 2022-04-19T00:54:32Z

examples/vite-react/jest.config.js

@@ -11,7 +11,7 @@ module.exports = {
  modulePaths: ['<rootDir>/src'],
  transform: {
    '^.+\\.(ts|js|tsx|jsx)$': '@swc/jest',
-    '^.+\\.css$': 'jest-preview/transforms/css',
+    '^.+\\.(css|scss|sass)$': 'jest-preview/transforms/css',


This comment is to remind not forget to update README.md on CSS transform

nvh95 · 2022-04-19T00:57:45Z

examples/vite-react/src/assets/_scss/partials/_partial-global-style.scss

@@ -0,0 +1,5 @@
+header {
+  .global-configured-sass {
+    font-size: 40px;


I personally think we can use text-transform: uppercase; (or add a background color) would make us easy to realize if @import takes place yet. Feel free to leave your option.

Similar to _partial-style.scss

nvh95 · 2022-04-19T01:01:01Z

examples/vite-react/src/assets/_scss/style.scss

@@ -0,0 +1,7 @@
+@import 'partials/partial-style';


As I understand @use and @import are pretty the same, @use is newer with some advantages. So I think we can use both @use and @import in our example to cover more Sass features.

nvh95 · 2022-04-19T01:04:03Z

src/configure.ts

+    // Create cache folder if it doesn't exist
+    if (!fs.existsSync(CACHE_FOLDER)) {
+      fs.mkdirSync(CACHE_FOLDER, {
+        recursive: true,
+      });
+    }


I think we can add an utils.ts file which has createCacheFolderIfNeeded() or something like that. We have this logic in quite many places.

…function

nvh95

Let's ship loadPaths and ~ in the next version.

# Conflicts: # examples/vite-react/package-lock.json

netlify · 2022-04-21T12:32:12Z

✅ Deploy Preview for jest-preview-library canceled.

Name	Link
🔨 Latest commit	`74d43d2`
🔍 Latest deploy log	https://app.netlify.com/sites/jest-preview-library/deploys/62614ec5ceebc6000989612d

ntt261298 added 3 commits April 16, 2022 23:36

feat: (In Progress) Support SASS

186147d

- Todo: Support @use, @import for SASS

refactor: Remove jest preview configure in setupTests

e52d4a1

refactor: Add partial scss style

6108ec8

ntt261298 linked an issue Apr 16, 2022 that may be closed by this pull request

Support SCSS #30

Closed

nvh95 reviewed Apr 16, 2022

View reviewed changes

ntt261298 and others added 12 commits April 17, 2022 08:57

fix: Move config jest-preview to setupFilesAfterEnv

f515658

fix: Use fs.existsSync to check if sassLoadPathsConfigPath exists

76e7d3f

fix: Move sass compile out of of run time phase

c8b853d

fix: Update code style for sass configure

290b8e4

feat: Add example for pre-configured sass file issue

0571ef6

chore: Add global scss style to main app

0ab3b65

Merge branch 'main' into sass-support

eca8a24

# Conflicts: # package-lock.json # package.json # src/configure.ts

chore: Update partial name for global scss

54e3428

chore: use sass.compile instead of sass.compileString

99de46e

Merge branch 'sass-support' of github.com:nvh95/jest-preview-dom into…

0a3057b

… sass-support

feat: Use CLI to transform sass in configure.ts, compile to transform…

32862e0

… sass in transform.ts

refactor: Update vite-react example to support global scss

0b2d56e

nvh95 reviewed Apr 18, 2022

View reviewed changes

nvh95 reviewed Apr 19, 2022

View reviewed changes

nvh95 and others added 7 commits April 19, 2022 21:47

docs: Update docs for sass

0a4aae1

fix: Correct css cache filenames

e7668b5

refactor: Use filename as the fallback for scss transformer

4761899

fix: Add createCacheFolderIfNeeded util, add comment to clarify exec …

baab8a5

…function

refactor: Update demo, vite example

1d4892c

Merge branch 'main' into sass-support

8225289

chore: release 0.1.5-alpha.0

9d44c67

ntt261298 marked this pull request as ready for review April 19, 2022 15:41

nvh95 mentioned this pull request Apr 21, 2022

Adapter for NextJS + NextJS example #60

Merged

6 tasks

nvh95 approved these changes Apr 21, 2022

View reviewed changes

Merge branch 'main' into sass-support

74d43d2

# Conflicts: # examples/vite-react/package-lock.json

nvh95 merged commit 08f8827 into main Apr 21, 2022

nvh95 deleted the sass-support branch April 21, 2022 13:13

nvh95 changed the title ~~feat: (In Progress) Support SASS~~ feat: Support SASS Apr 21, 2022

This was referenced Apr 23, 2022

Support CSS Modules through jestPreviewConfigure, externalCss option #71

Closed

Support loadPath and import ~ for Sass #72

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support SASS #52

feat: Support SASS #52

ntt261298 commented Apr 16, 2022 •

edited by nvh95

nvh95 Apr 16, 2022

ntt261298 Apr 17, 2022

nvh95 Apr 16, 2022

ntt261298 Apr 17, 2022

nvh95 Apr 16, 2022

ntt261298 Apr 17, 2022

nvh95 Apr 16, 2022

ntt261298 Apr 17, 2022

nvh95 Apr 16, 2022

ntt261298 Apr 17, 2022

nvh95 Apr 16, 2022

nvh95 Apr 18, 2022

nvh95 Apr 18, 2022

nvh95 Apr 18, 2022

nvh95 Apr 18, 2022

nvh95 left a comment

nvh95 Apr 19, 2022

nvh95 Apr 19, 2022

nvh95 Apr 19, 2022

nvh95 Apr 19, 2022

nvh95 Apr 19, 2022

nvh95 Apr 19, 2022

nvh95 left a comment

netlify bot commented Apr 21, 2022 •

edited

		if (cssFile.endsWith('.scss') \|\| cssFile.endsWith('.sass')) {
		const sass = require('sass');

	const destinationBasename = `cache-${basename.replace('/', delimiter)}`;
	const destinationBasename = `cache-${basename.replace(/\//g, delimiter)}`;

feat: Support SASS #52

feat: Support SASS #52

Conversation

ntt261298 commented Apr 16, 2022 • edited by nvh95

Features

Todo:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvh95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvh95 left a comment

Choose a reason for hiding this comment

netlify bot commented Apr 21, 2022 • edited

✅ Deploy Preview for jest-preview-library canceled.

ntt261298 commented Apr 16, 2022 •

edited by nvh95

netlify bot commented Apr 21, 2022 •

edited