Skip to content

fix(css): sass rebase url in relative imported modules #20067

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions docs/config/shared-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ Note if an inline config is provided, Vite will not search for other PostCSS con
Specify options to pass to CSS pre-processors. The file extensions are used as keys for the options. The supported options for each preprocessor can be found in their respective documentation:

- `sass`/`scss`:
- Select the sass API to use with `api: "modern-compiler" | "modern"` (default `"modern-compiler"` if `sass-embedded` is installed, otherwise `"modern"`). For the best performance, it's recommended to use `api: "modern-compiler"` with the `sass-embedded` package.
- Uses `sass-embedded` if installed, otherwise uses `sass`. For the best performance, it's recommended to install the `sass-embedded` package.
- [Options](https://sass-lang.com/documentation/js-api/interfaces/stringoptions/)
- `less`: [Options](https://lesscss.org/usage/#less-options).
- `styl`/`stylus`: Only [`define`](https://stylus-lang.com/docs/js.html#define-name-node) is supported, which can be passed as an object.
Expand All @@ -247,7 +247,6 @@ export default defineConfig({
},
},
scss: {
api: 'modern-compiler', // or "modern"
importers: [
// ...
],
Expand Down
179 changes: 17 additions & 162 deletions packages/vite/src/node/plugins/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2268,7 +2268,7 @@ type PreprocessorAdditionalData =

export type SassPreprocessorOptions = {
additionalData?: PreprocessorAdditionalData
} & ({ api?: 'modern' | 'modern-compiler' } & SassModernPreprocessBaseOptions)
} & SassModernPreprocessBaseOptions

export type LessPreprocessorOptions = {
additionalData?: PreprocessorAdditionalData
Expand Down Expand Up @@ -2392,156 +2392,31 @@ function cleanScssBugUrl(url: string) {

// #region Sass
// .scss/.sass processor
const makeModernScssWorker = (
const makeScssWorker = (
environment: PartialEnvironment,
resolvers: CSSAtImportResolvers,
alias: Alias[],
maxWorkers: number | undefined,
_maxWorkers: number | undefined,
) => {
const internalCanonicalize = async (
url: string,
importer: string,
): Promise<string | null> => {
importer = cleanScssBugUrl(importer)
const resolved = await resolvers.sass(environment, url, importer)
return resolved ?? null
}

const skipRebaseUrls = (unquotedUrl: string, rawUrl: string) => {
const isQuoted = rawUrl[0] === '"' || rawUrl[0] === "'"
// matches `url($foo)`
if (!isQuoted && unquotedUrl[0] === '$') {
return true
}
// matches `url(#{foo})` and `url('#{foo}')`
return unquotedUrl.startsWith('#{')
}

const internalLoad = async (file: string, rootFile: string) => {
const result = await rebaseUrls(
environment,
file,
rootFile,
alias,
resolvers.sass,
skipRebaseUrls,
)
if (result.contents) {
return result.contents
}
return await fsp.readFile(result.file, 'utf-8')
}
let compilerPromise: Promise<Sass.AsyncCompiler> | undefined

const worker = new WorkerWithFallback(
() =>
async (
// we use the compiler api provided by sass
// instead of creating a worker pool on our own
type WorkerType = InstanceType<
typeof WorkerWithFallback<
[
sassPath: string,
data: string,
// additionalData can a function that is not cloneable but it won't be used
options: SassStylePreprocessorInternalOptions & {
api: 'modern'
additionalData: undefined
},
) => {
// eslint-disable-next-line no-restricted-globals -- this function runs inside a cjs worker
const sass: typeof Sass = require(sassPath)
// eslint-disable-next-line no-restricted-globals
const path: typeof import('node:path') = require('node:path')

const { fileURLToPath, pathToFileURL }: typeof import('node:url') =
// eslint-disable-next-line no-restricted-globals
require('node:url')

const sassOptions = { ...options } as Sass.StringOptions<'async'>
sassOptions.url = pathToFileURL(options.filename)
sassOptions.sourceMap = options.enableSourcemap

const internalImporter: Sass.Importer<'async'> = {
async canonicalize(url, context) {
const importer = context.containingUrl
? fileURLToPath(context.containingUrl)
: options.filename
const resolved = await internalCanonicalize(url, importer)
if (
resolved &&
// only limit to these extensions because:
// - for the `@import`/`@use`s written in file loaded by `load` function,
// the `canonicalize` function of that `importer` is called first
// - the `load` function of an importer is only called for the importer
// that returned a non-null result from its `canonicalize` function
(resolved.endsWith('.css') ||
resolved.endsWith('.scss') ||
resolved.endsWith('.sass'))
) {
return pathToFileURL(resolved)
}
return null
},
async load(canonicalUrl) {
const ext = path.extname(canonicalUrl.pathname)
let syntax: Sass.Syntax = 'scss'
if (ext === '.sass') {
syntax = 'indented'
} else if (ext === '.css') {
syntax = 'css'
}
const contents = await internalLoad(
fileURLToPath(canonicalUrl),
options.filename,
)
return { contents, syntax, sourceMapUrl: canonicalUrl }
},
}
sassOptions.importers = [
...(sassOptions.importers ?? []),
internalImporter,
]

const result = await sass.compileStringAsync(data, sassOptions)
return {
css: result.css,
map: result.sourceMap ? JSON.stringify(result.sourceMap) : undefined,
stats: {
includedFiles: result.loadedUrls
.filter((url) => url.protocol === 'file:')
.map((url) => fileURLToPath(url)),
},
} satisfies ScssWorkerResult
},
{
parentFunctions: {
internalCanonicalize,
internalLoad,
},
shouldUseFake(_sassPath, _data, options) {
// functions and importer is a function and is not serializable
// in that case, fallback to running in main thread
return !!(
(options.functions && Object.keys(options.functions).length > 0) ||
(options.importers &&
(!Array.isArray(options.importers) ||
options.importers.length > 0)) ||
options.logger
)
},
max: maxWorkers,
},
)
return worker
}

// this is mostly a copy&paste of makeModernScssWorker
// however sharing code between two is hard because
// makeModernScssWorker above needs function inlined for worker.
const makeModernCompilerScssWorker = (
environment: PartialEnvironment,
resolvers: CSSAtImportResolvers,
alias: Alias[],
_maxWorkers: number | undefined,
) => {
let compilerPromise: Promise<Sass.AsyncCompiler> | undefined
],
ScssWorkerResult
>
>

const worker: Awaited<ReturnType<typeof makeModernScssWorker>> = {
const worker: WorkerType = {
async run(sassPath, data, options) {
// need pathToFileURL for windows since import("D:...") fails
// https://github.com/nodejs/node/issues/31710
Expand Down Expand Up @@ -2609,6 +2484,7 @@ const makeModernCompilerScssWorker = (
...(sassOptions.importers ?? []),
internalImporter,
]
sassOptions.importer ??= internalImporter

const result = await compiler.compileStringAsync(data, sassOptions)
return {
Expand Down Expand Up @@ -2639,12 +2515,7 @@ type ScssWorkerResult = {
const scssProcessor = (
maxWorkers: number | undefined,
): StylePreprocessor<SassStylePreprocessorInternalOptions> => {
const workerMap = new Map<
unknown,
ReturnType<
typeof makeModernScssWorker | typeof makeModernCompilerScssWorker
>
>()
const workerMap = new Map<unknown, ReturnType<typeof makeScssWorker>>()

return {
close() {
Expand All @@ -2654,26 +2525,11 @@ const scssProcessor = (
},
async process(environment, source, root, options, resolvers) {
const sassPackage = loadSassPackage(root)
const api =
options.api ??
(sassPackage.name === 'sass-embedded' ? 'modern-compiler' : 'modern')

if (!workerMap.has(options.alias)) {
workerMap.set(
options.alias,
api === 'modern-compiler'
? makeModernCompilerScssWorker(
environment,
resolvers,
options.alias,
maxWorkers,
)
: makeModernScssWorker(
environment,
resolvers,
options.alias,
maxWorkers,
),
makeScssWorker(environment, resolvers, options.alias, maxWorkers),
)
}
const worker = workerMap.get(options.alias)!
Expand All @@ -2693,7 +2549,6 @@ const scssProcessor = (
const result = await worker.run(
sassPackage.path,
data,
// @ts-expect-error the correct worker is selected for `options.type`
optionsWithoutAdditionalData,
)
const deps = result.stats.includedFiles.map((f) => cleanScssBugUrl(f))
Expand Down

This file was deleted.

15 changes: 0 additions & 15 deletions playground/css-sourcemap/vite.config-sass-modern.js

This file was deleted.

6 changes: 0 additions & 6 deletions playground/css/__tests__/sass-modern/sass-modern.spec.ts

This file was deleted.

5 changes: 5 additions & 0 deletions playground/css/__tests__/sass-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const sassTest = () => {
const imported = await page.$('.sass')
const atImport = await page.$('.sass-at-import')
const atImportAlias = await page.$('.sass-at-import-alias')
const atImportRelative = await page.$('.sass-at-import-relative')
const urlStartsWithVariable = await page.$('.sass-url-starts-with-variable')
const urlStartsWithVariableInterpolation1 = await page.$(
'.sass-url-starts-with-interpolation1',
Expand All @@ -38,6 +39,10 @@ export const sassTest = () => {
expect(await getBg(atImportAlias)).toMatch(
isBuild ? /base64/ : '/nested/icon.png',
)
expect(await getColor(atImportRelative)).toBe('olive')
expect(await getBg(atImportRelative)).toMatch(
isBuild ? /base64/ : '/nested/icon.png',
)
expect(await getBg(urlStartsWithVariable)).toMatch(
isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
)
Expand Down
3 changes: 3 additions & 0 deletions playground/css/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ <h1>CSS</h1>
@import from SASS _index: This should be olive and have bg image which url
contains alias
</p>
<p class="sass-at-import-relative">
@import from SASS relative: This should be olive and have bg image
</p>
<p class="sass-partial">@import from SASS _partial: This should be orchid</p>
<p class="sass-url-starts-with-variable">url starts with variable</p>
<p class="sass-url-starts-with-interpolation1">
Expand Down
4 changes: 4 additions & 0 deletions playground/css/nested/relative.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.sass-at-import-relative {
color: olive;
background: url(./icon.png) 10px no-repeat;
}
1 change: 1 addition & 0 deletions playground/css/sass.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
@use '=/nested'; // alias + custom index resolving -> /nested/_index.scss
@use '=/nested/partial'; // sass convention: omitting leading _ for partials
@use './nested/relative'; // relative path
@use '@vitejs/test-css-dep'; // package w/ sass entry points
@use '@vitejs/test-css-dep-exports'; // package with a sass export mapping
@use '@vitejs/test-scss-proxy-dep'; // package with a sass proxy import
Expand Down
7 changes: 0 additions & 7 deletions playground/css/vite.config-sass-modern-compiler-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,4 @@ export default defineConfig({
},
},
},
css: {
preprocessorOptions: {
scss: {
api: 'modern-compiler',
},
},
},
})
18 changes: 0 additions & 18 deletions playground/css/vite.config-sass-modern.js

This file was deleted.

3 changes: 1 addition & 2 deletions playground/vitestGlobalSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ export async function setup({ provide }: TestProject): Promise<void> {
})
// also setup dedicated copy for "variant" tests
for (const [original, variants] of [
['css', ['sass-modern', 'lightningcss']],
['css-sourcemap', ['sass-modern']],
['css', ['lightningcss']],
['transform-plugin', ['base']],
] as const) {
for (const variant of variants) {
Expand Down