-
-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
perf: reuse compiler process when using sass-embedded #1195
Changes from 3 commits
ecc9206
5952f5a
2e8a443
b111ffd
e75dbde
2514e16
ea88129
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,7 +174,8 @@ async function getSassOptions( | |
}; | ||
} | ||
|
||
const isModernAPI = loaderOptions.api === "modern"; | ||
const isModernAPI = | ||
loaderOptions.api === "modern" || loaderOptions.api === "modern-compiler"; | ||
const { resourcePath } = loaderContext; | ||
|
||
if (isModernAPI) { | ||
|
@@ -650,20 +651,21 @@ function getWebpackImporter(loaderContext, implementation, includePaths) { | |
} | ||
|
||
let nodeSassJobQueue = null; | ||
const sassModernCompilers = {}; | ||
|
||
/** | ||
* Verifies that the implementation and version of Sass is supported by this loader. | ||
* | ||
* @param {Object} loaderContext | ||
* @param {Object} implementation | ||
* @param {Object} options | ||
* @returns {Function} | ||
*/ | ||
function getCompileFn(implementation, options) { | ||
const isNewSass = | ||
implementation.info.includes("dart-sass") || | ||
implementation.info.includes("sass-embedded"); | ||
function getCompileFn(loaderContext, implementation, options) { | ||
const isDartSass = implementation.info.includes("dart-sass"); | ||
const isSassEmbedded = implementation.info.includes("sass-embedded"); | ||
|
||
if (isNewSass) { | ||
if (isDartSass || isSassEmbedded) { | ||
if (options.api === "modern") { | ||
return (sassOptions) => { | ||
const { data, ...rest } = sassOptions; | ||
|
@@ -672,6 +674,32 @@ function getCompileFn(implementation, options) { | |
}; | ||
} | ||
|
||
if (options.api === "modern-compiler") { | ||
return async (sassOptions) => { | ||
// eslint-disable-next-line no-underscore-dangle | ||
const webpackCompiler = loaderContext._compiler; | ||
const { data, ...rest } = sassOptions; | ||
|
||
// Some people can run the loader in a multi-threading way; | ||
// there is no webpack compiler object in such case. | ||
if (webpackCompiler) { | ||
const key = isDartSass ? "dart-sass" : "sass-embedded"; | ||
if (!sassModernCompilers[key]) { | ||
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'm not sure if 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 see, let's use |
||
// Create a long-running compiler process that can be reused | ||
// for compiling individual files. | ||
const compiler = await implementation.initAsyncCompiler(); | ||
webpackCompiler.hooks.shutdown.tap("sass-loader", () => { | ||
compiler.dispose(); | ||
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. In my debugging this hook was never called; not sure when it should be. In any case, should we 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. After |
||
}); | ||
sassModernCompilers[key] = compiler; | ||
} | ||
return sassModernCompilers[key].compileStringAsync(data, rest); | ||
} | ||
|
||
return implementation.compileStringAsync(data, rest); | ||
}; | ||
} | ||
|
||
return (sassOptions) => | ||
new Promise((resolve, reject) => { | ||
implementation.render(sassOptions, (error, result) => { | ||
|
@@ -686,7 +714,7 @@ function getCompileFn(implementation, options) { | |
}); | ||
} | ||
|
||
if (options.api === "modern") { | ||
if (options.api === "modern" || options.api === "modern-compiler") { | ||
throw new Error("Modern API is not supported for 'node-sass'"); | ||
} | ||
|
||
|
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.
Removed unused argument.