Skip to content

Commit

Permalink
Capture sass warnings, emit them as esbuild warnings
Browse files Browse the repository at this point in the history
Sass just logs warnings as they occur. This is a bit of a sub-par
experience with esbuild, and it would be nicer if the warnings were
emitted by esbuild as it does other warnings/errors.

There is some awkwardness to capturing the warnings: sass's API seems a
bit stuck in some pre-async design patterns, and having a separate
logger that gets called with syntax warnings rather than returning them
as part of the compile result is one symptom of that.

Accomplishing this with config outside the plugin is feasible but
annoying. It requires writing a wrapper plugin around `sassPlugin` that
wraps the `onLoad` handler to append captured warnings, plus an
`onStart` handler to clear warnings (for watch mode). Additionally, if
the build config has multiple entrypoints it's impossible to tell which
file a warning in an entrypoint came from: warnings directly within an
entrypoint don't have a `url` on the warning message (since this plugin
reads the file and passes the contents to `sass.renderString`).
Implemented within the plugin's renderer, this can be worked around
easily since there's only ever one entrypoint in scope.

So I think the cleanest way to capture this data is to construct the
logger & store the warnings as close to the `sass.renderString` call as
reasonable. My hope is others will also see this as a useful default
behavior!
  • Loading branch information
wfleming committed Apr 20, 2023
1 parent f2e99b0 commit c7fbd0d
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 4 deletions.
7 changes: 6 additions & 1 deletion src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ export function sassPlugin(options: SassPluginOptions = {}): Plugin {

onLoad({filter: options.filter ?? DEFAULT_FILTER}, useCache(options, async path => {
try {
let {cssText, watchFiles} = renderSync(path)
let {cssText, watchFiles, warnings} = renderSync(path)
if (!warnings) {
warnings = []
}

watched[path] = watchFiles

Expand Down Expand Up @@ -108,11 +111,13 @@ export function sassPlugin(options: SassPluginOptions = {}): Plugin {
contents: cssText,
loader: 'css',
resolveDir,
warnings,
watchFiles
} : {
contents: makeModule(cssText, type, nonce),
loader: 'js',
resolveDir,
warnings,
watchFiles
}

Expand Down
30 changes: 30 additions & 0 deletions src/render.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {dirname, parse, relative, resolve, sep} from 'path'
import fs, {readFileSync} from 'fs'
import {createResolver, fileSyntax, sourceMappingURL} from './utils'
import {PartialMessage} from 'esbuild'
import * as sass from 'sass'
import {ImporterResult} from 'sass'
import {fileURLToPath, pathToFileURL} from 'url'
Expand All @@ -11,6 +12,7 @@ export type RenderSync = (path: string) => RenderResult
export type RenderResult = {
cssText: string
watchFiles: string[]
warnings?: PartialMessage[]
}

export function createRenderer(options: SassPluginOptions = {}, sourcemap: boolean): RenderSync {
Expand Down Expand Up @@ -80,13 +82,40 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole
options.url = pathToFileURL(path)
}

const warnings: PartialMessage[] = []
const logger = options.logger ?? {
warn: function (message, opts) {
if (!opts.span) {
warnings.push({ text: `sass warning: ${message}` })
} else {
const filename = opts.span.url?.pathname ?? path
const esbuildMsg = {
text: message,
location: {
file: filename,
line: opts.span.start.line,
column: opts.span.start.column,
lineText: opts.span.text,
},
detail: {
deprecation: opts.deprecation,
stack: opts.stack,
}
}

warnings.push(esbuildMsg)
}
}
}

const {
css,
loadedUrls,
sourceMap
} = sass.compileString(source, {
sourceMapIncludeSources: true,
...options,
logger,
syntax,
importer: {
load(canonicalUrl: URL): ImporterResult | null {
Expand Down Expand Up @@ -147,6 +176,7 @@ export function createRenderer(options: SassPluginOptions = {}, sourcemap: boole

return {
cssText,
warnings: warnings,
watchFiles: [path, ...loadedUrls.map(fileURLToPath)]
}
}
Expand Down
1 change: 1 addition & 0 deletions test/fixture/warnings/_partial.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
div
7 changes: 7 additions & 0 deletions test/fixture/warnings/index.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
body
font-size: 12px

p

a
color: red
2 changes: 2 additions & 0 deletions test/fixture/warnings/index2.sass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@import "./index"
@import "./partial"
72 changes: 69 additions & 3 deletions test/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('unit tests', function () {
writeTextFile('./index.sass', readTextFile('./index-v2.sass'))
await ctx.rebuild()
expect(fs.readFileSync('./out/index.css', 'utf-8').replace(/\/\*.+\*\//g, '')).to.equalIgnoreSpaces(`
body { background-color: red; }
body { background-color: red; }
body { font: 99% "Times New Roman", serif; color: #666; }
`)

Expand All @@ -106,7 +106,7 @@ describe('unit tests', function () {
writeTextFile('./dependency.sass', readTextFile('./dependency-v3.sass'))
await ctx.rebuild()
expect(fs.readFileSync('./out/index.css', 'utf-8').replace(/\/\*.+\*\//g, '')).to.equalIgnoreSpaces(`
body { background-color: blue; }
body { background-color: blue; }
body { font: 99% "Times New Roman", serif; color: #666; }
`)

Expand Down Expand Up @@ -152,4 +152,70 @@ describe('unit tests', function () {
expect(readTextFile('out/index.js')).to.equalIgnoreSpaces(readTextFile('snapshot.js'))
})

})
it('captures warnings in entrypoint', async function() {
const options = useFixture('warnings')
let warnings = []

await esbuild.build({
...options,
entryPoints: ['./index.sass'],
logLevel: 'silent',
outdir: './out',
bundle: true,
plugins: [
sassPlugin({ syntax: 'nested' }),
{
name: "capture-build-end-warnings",
setup: function (build) {
build.onEnd(async function(result) {
warnings = result.warnings
})
}
}
]
})

expect(warnings.length).to.equal(1)

expect(warnings[0].text).to.include("This selector doesn't have any properties")
expect(warnings[0].location.file).to.equal("index.sass")
expect(warnings[0].location.line).to.equal(3)
expect(warnings[0].location.lineText).to.equal("p")
})

it('captures warnings in imports', async function() {
const options = useFixture('warnings')
let warnings = []

await esbuild.build({
...options,
bundle: true,
entryPoints: ['./index2.sass'],
logLevel: 'silent',
outdir: './out',
plugins: [
sassPlugin({ syntax: 'nested' }),
{
name: "capture-build-end-warnings",
setup: function (build) {
build.onEnd(async function(result) {
warnings = result.warnings
})
}
}
]
})

expect(warnings.length).to.equal(2)

const indexWarning = warnings.find(w => w.location.file === "index.sass")
expect(indexWarning.text).to.include("This selector doesn't have any properties")
expect(indexWarning.location.line).to.equal(3)
expect(indexWarning.location.lineText).to.equal("p")

const partialWarning = warnings.find(w => w.location.file === "_partial.sass")
expect(partialWarning.text).to.include("This selector doesn't have any properties")
expect(partialWarning.location.line).to.equal(0)
expect(partialWarning.location.lineText).to.equal("div")
})
})

0 comments on commit c7fbd0d

Please sign in to comment.