-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
feat: Support SASS #52
Changes from 15 commits
186147d
e52d4a1
6108ec8
f515658
76e7d3f
c8b853d
290b8e4
0571ef6
0ab3b65
eca8a24
54e3428
99de46e
0a3057b
32862e0
0b2d56e
0a4aae1
e7668b5
4761899
baab8a5
1d4892c
8225289
9d44c67
74d43d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
@import 'partials/partial-global-style'; | ||
|
||
header { | ||
.global-configured-sass { | ||
color: yellow; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
header { | ||
.global-configured-sass { | ||
font-size: 40px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally think we can use Similar to _partial-style.scss |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
header { | ||
.imported-sass { | ||
font-size: 40px; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
@import 'partials/partial-style'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I understand |
||
|
||
header { | ||
.imported-sass { | ||
color: pink; | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,17 +1,14 @@ | ||||||
import path from 'path'; | ||||||
import fs from 'fs'; | ||||||
|
||||||
const CACHE_FOLDER = './node_modules/.cache/jest-preview-dom'; | ||||||
import { CACHE_FOLDER } from './constants'; | ||||||
|
||||||
interface JestPreviewConfigOptions { | ||||||
externalCss?: string[]; | ||||||
externalCss: string[]; | ||||||
publicFolder?: string; | ||||||
} | ||||||
|
||||||
export async function jestPreviewConfigure( | ||||||
options: JestPreviewConfigOptions = { | ||||||
externalCss: [], | ||||||
}, | ||||||
options: JestPreviewConfigOptions = { externalCss: [] }, | ||||||
) { | ||||||
if (!fs.existsSync('./node_modules/.cache/jest-preview-dom')) { | ||||||
fs.mkdirSync('./node_modules/.cache/jest-preview-dom', { | ||||||
|
@@ -21,13 +18,41 @@ export async function jestPreviewConfigure( | |||||
|
||||||
options.externalCss?.forEach((cssFile) => { | ||||||
const basename = path.basename(cssFile); | ||||||
|
||||||
// Avoid name collision | ||||||
// Add `global` to let `jest-preview` server that we want to cache those files | ||||||
const destinationBasename = `cache-${basename}`; | ||||||
const destinationFile = path.join( | ||||||
'./node_modules/.cache/jest-preview-dom', | ||||||
destinationBasename, | ||||||
); | ||||||
// Example: src/common/styles.css => cache-src___common___styles.css | ||||||
const delimiter = '___'; | ||||||
const destinationBasename = `cache-${basename.replace('/', delimiter)}`; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will replace the first
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this logic correct since we are using |
||||||
const destinationFile = path.join(CACHE_FOLDER, destinationBasename); | ||||||
|
||||||
// Create cache folder if it doesn't exist | ||||||
if (!fs.existsSync(CACHE_FOLDER)) { | ||||||
fs.mkdirSync(CACHE_FOLDER, { | ||||||
recursive: true, | ||||||
}); | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can add an |
||||||
|
||||||
// If sass file is included, we need to transform it to css | ||||||
if (cssFile.endsWith('.scss') || cssFile.endsWith('.sass')) { | ||||||
const { exec } = require('child_process'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can use |
||||||
|
||||||
const cssDestinationFile = destinationFile.replace( | ||||||
/\.(scss|sass)$/, | ||||||
'.css', | ||||||
); | ||||||
|
||||||
// Transform sass to css and save to cache folder | ||||||
exec( | ||||||
`./node_modules/.bin/sass ${cssFile} ${cssDestinationFile} --no-source-map`, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a little bad feeling about using I'm not sure, since this is pretty similar to the problem I have to solve when adding No clear action items now. I think just to keep this implementation in mind for now. |
||||||
(err: any) => { | ||||||
if (err) { | ||||||
console.log(err); | ||||||
} | ||||||
}, | ||||||
); | ||||||
return; | ||||||
} | ||||||
Comment on lines
+43
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||||||
|
||||||
// TODO: To move to load file directly instead of cloning them to `.cache` | ||||||
// Move together with transform | ||||||
// TODO: To cache those files. We cannot cache them by checking if files exists | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const CACHE_FOLDER = './node_modules/.cache/jest-preview-dom'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is to remind not forget to update README.md on CSS transform