Skip to content
This repository has been archived by the owner on Nov 22, 2024. It is now read-only.

Commit

Permalink
spike: async rendering
Browse files Browse the repository at this point in the history
we have a lot of entrypoints/css bundles in our app, and in adding them
all I realized the build was getting a lot slower and, suspiciously, all
the sass builds took the exact same amount of time to build. Seemed like
a symptom using sync interfaces could cause, so I thought I'd try seeing
if async helped.

It did not! [Dart's docs][0] do say upfront that the async interface can
be ~2x slower, but I hoped that there'd still be a win on overall
throughput even if individual bundles were slower. For some bundles that
was the case: previously every css bundle was about ~8.6s, some smaller
ones are now ~500ms. However, the slow ones got a lot more than 2x
slower: a few are now ~18.5s, which is longer than the previous entire
build (including JS), so overall wall-clock time of my entire build went
up from ~11s to ~20s.

So this went nowhere. It's possible there's still some synchronous
chokepoints in the plugin I didn't see, but I'm out of time on this
right now. If this had been more promising I would have put in more work
to make it an option for the plugin. Committing it anyway as a
historical artifact. libsass's poor async performance seems like it must
be a design issue of some kind, so maybe they'll improve that in the
future.

[0]: https://sass-lang.com/documentation/js-api/modules#compileStringAsync
  • Loading branch information
wfleming committed Jun 13, 2023
1 parent aaf85aa commit e972451
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {sassPlugin} from './plugin'

export type Type = 'css' | 'style' | 'css-text' | 'lit-css'

export type SassPluginOptions = StringOptions<'sync'> & {
export type SassPluginOptions = StringOptions<'async'> & {

/**
* Careful: this RegExp has to respect Go limitations!!!
Expand Down
4 changes: 2 additions & 2 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ export function sassPlugin(options: SassPluginOptions = {}): Plugin {
}))
}

const renderSync = createRenderer(options, options.sourceMap ?? sourcemap)
const renderAsync = createRenderer(options, options.sourceMap ?? sourcemap)

onLoad({filter: options.filter ?? DEFAULT_FILTER}, useCache(options, async path => {
try {
let {cssText, watchFiles, warnings} = renderSync(path)
let {cssText, watchFiles, warnings} = await renderAsync(path)
if (!warnings) {
warnings = []
}
Expand Down
49 changes: 29 additions & 20 deletions src/render.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {dirname, parse, relative, resolve, sep} from 'path'
import fs, {readFileSync} from 'fs'
import fs from "node:fs/promises"
import {createResolver, fileSyntax, sourceMappingURL} from './utils'
import {PartialMessage} from 'esbuild'
import * as sass from 'sass'
import {ImporterResult} from 'sass'
import {fileURLToPath, pathToFileURL} from 'url'
import {SassPluginOptions} from './index'

export type RenderSync = (path: string) => RenderResult
export type RenderSync = (path: string) => Promise<RenderResult>

export type RenderResult = {
cssText: string
Expand All @@ -20,36 +20,45 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole
const loadPaths = options.loadPaths!
const resolveModule = createResolver(options, loadPaths)

async function fsExists(path) {
try {
await fs.stat(path)
return true
} catch {
return false
}
}

/**
* NOTE: we're deliberately ignoring sass recommendation to avoid sacrificing speed here!
* - we prefer fragment attempt over syntax attempt
* - we prefer .scss and .css over .sass
* - we don't throw exceptions if the URL is ambiguous
*/
function resolveImport(pathname: string, ext?: string): string | null {
async function resolveImport(pathname: string, ext?: string): Promise<string | null> {
if (ext) {
let filename = pathname + ext
if (fs.existsSync(filename)) {
if (await fsExists(filename)) {
return filename
}
const index = filename.lastIndexOf(sep)
filename = index >= 0 ? filename.slice(0, index) + sep + '_' + filename.slice(index + 1) : '_' + filename
if (fs.existsSync(filename)) {
if (await fsExists(filename)) {
return filename
}
return null
} else {
if (!fs.existsSync(dirname(pathname))) {
if (!await fsExists(dirname(pathname))) {
return null
}
return resolveImport(pathname, '.scss')
|| resolveImport(pathname, '.css')
|| resolveImport(pathname, '.sass')
|| resolveImport(pathname + sep + 'index')
return await resolveImport(pathname, '.scss')
|| await resolveImport(pathname, '.css')
|| await resolveImport(pathname, '.sass')
|| await resolveImport(pathname + sep + 'index')
}
}

function resolveRelativeImport(loadPath: string, filename: string): string | null {
async function resolveRelativeImport(loadPath: string, filename: string): Promise<string | null> {
const absolute = resolve(loadPath, filename)
const pathParts = parse(absolute)
if (pathParts.ext) {
Expand All @@ -64,18 +73,18 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole
/**
* renderSync
*/
return function (path: string): RenderResult {
return async function (path: string): Promise<RenderResult> {

const basedir = dirname(path)

let source = fs.readFileSync(path, 'utf-8')
let source = await fs.readFile(path, 'utf-8')
if (options.precompile) {
source = options.precompile(source, path, true)
}

const syntax = fileSyntax(path)
if (syntax === 'css') {
return {cssText: readFileSync(path, 'utf-8'), watchFiles: [path]}
return {cssText: await fs.readFile(path, 'utf-8'), watchFiles: [path]}
}

if (options.quietDeps) {
Expand Down Expand Up @@ -112,15 +121,15 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole
css,
loadedUrls,
sourceMap
} = sass.compileString(source, {
} = await sass.compileStringAsync(source, {
sourceMapIncludeSources: true,
...options,
logger,
syntax,
importer: {
load(canonicalUrl: URL): ImporterResult | null {
async load(canonicalUrl: URL): Promise<ImporterResult | null> {
const pathname = fileURLToPath(canonicalUrl)
let contents = fs.readFileSync(pathname, 'utf8')
let contents = await fs.readFile(pathname, 'utf8')
if (options.precompile) {
contents = options.precompile(contents, pathname, false)
}
Expand All @@ -130,7 +139,7 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole
sourceMapUrl: sourcemap ? canonicalUrl : undefined
}
},
canonicalize(url: string): URL | null {
async canonicalize(url: string): Promise<URL | null> {
let filename: string
if (url.startsWith('~')) {
filename = resolveModule(decodeURI(url.slice(1)), basedir)
Expand All @@ -148,12 +157,12 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole
if (options.importMapper) {
filename = options.importMapper(filename)
}
let resolved = resolveRelativeImport(basedir, filename)
let resolved = await resolveRelativeImport(basedir, filename)
if (resolved) {
return pathToFileURL(resolved)
}
for (const loadPath of loadPaths) {
resolved = resolveRelativeImport(loadPath, filename)
resolved = await resolveRelativeImport(loadPath, filename)
if (resolved) {
return pathToFileURL(resolved)
}
Expand Down

0 comments on commit e972451

Please sign in to comment.