Skip to content
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

fix(css): url() with variable in sass/less (fixes #3644, #7651) #10741

Merged
merged 5 commits into from Nov 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions packages/vite/src/node/plugins/css.ts
Expand Up @@ -1493,7 +1493,7 @@ const scss: SassStylePreprocessor = async (
const internalImporter: Sass.Importer = (url, importer, done) => {
resolvers.sass(url, importer).then((resolved) => {
if (resolved) {
rebaseUrls(resolved, options.filename, options.alias)
rebaseUrls(resolved, options.filename, options.alias, '$')
.then((data) => done?.(data))
.catch((data) => done?.(data))
} else {
Expand Down Expand Up @@ -1577,7 +1577,8 @@ const sass: SassStylePreprocessor = (source, root, options, aliasResolver) =>
async function rebaseUrls(
file: string,
rootFile: string,
alias: Alias[]
alias: Alias[],
variablePrefix: string
): Promise<Sass.ImporterReturnType> {
file = path.resolve(file) // ensure os-specific flashes
// in the same dir, no need to rebase
Expand All @@ -1602,6 +1603,8 @@ async function rebaseUrls(
let rebased
const rebaseFn = (url: string) => {
if (url.startsWith('/')) return url
// ignore url's starting with variable
if (url.startsWith(variablePrefix)) return url
// match alias, no need to rewrite
for (const { find } of alias) {
const matches =
Expand Down Expand Up @@ -1734,7 +1737,12 @@ function createViteLessPlugin(
path.join(dir, '*')
)
if (resolved) {
const result = await rebaseUrls(resolved, this.rootFile, this.alias)
const result = await rebaseUrls(
resolved,
this.rootFile,
this.alias,
'@'
)
let contents: string
if (result && 'contents' in result) {
contents = result.contents
Expand Down
9 changes: 9 additions & 0 deletions playground/css/__tests__/css.spec.ts
Expand Up @@ -10,6 +10,7 @@ import {
removeFile,
serverLogs,
untilUpdated,
viteTestUrl,
withRetry
} from '~utils'

Expand Down Expand Up @@ -78,6 +79,7 @@ test('sass', async () => {
const imported = await page.$('.sass')
const atImport = await page.$('.sass-at-import')
const atImportAlias = await page.$('.sass-at-import-alias')
const urlStartsWithVariable = await page.$('.sass-url-starts-with-variable')
const partialImport = await page.$('.sass-partial')

expect(await getColor(imported)).toBe('orange')
Expand All @@ -87,6 +89,9 @@ test('sass', async () => {
expect(await getBg(atImportAlias)).toMatch(
isBuild ? /base64/ : '/nested/icon.png'
)
expect(await getBg(urlStartsWithVariable)).toMatch(
isBuild ? /ok\.\w+\.png/ : `${viteTestUrl}/ok.png`
)
expect(await getColor(partialImport)).toBe('orchid')

editFile('sass.scss', (code) =>
Expand All @@ -109,6 +114,7 @@ test('less', async () => {
const imported = await page.$('.less')
const atImport = await page.$('.less-at-import')
const atImportAlias = await page.$('.less-at-import-alias')
const urlStartsWithVariable = await page.$('.less-url-starts-with-variable')

expect(await getColor(imported)).toBe('blue')
expect(await getColor(atImport)).toBe('darkslateblue')
Expand All @@ -117,6 +123,9 @@ test('less', async () => {
expect(await getBg(atImportAlias)).toMatch(
isBuild ? /base64/ : '/nested/icon.png'
)
expect(await getBg(urlStartsWithVariable)).toMatch(
isBuild ? /ok\.\w+\.png/ : `${viteTestUrl}/ok.png`
)

editFile('less.less', (code) => code.replace('@color: blue', '@color: red'))
await untilUpdated(() => getColor(imported), 'red')
Expand Down
6 changes: 3 additions & 3 deletions playground/css/composes-path-resolving.module.css
@@ -1,11 +1,11 @@
.path-resolving-css {
composes: apply-color from '@/composed.module.css';
composes: apply-color from '=/composed.module.css';
}

.path-resolving-sass {
composes: apply-color from '@/composed.module.scss';
composes: apply-color from '=/composed.module.scss';
}

.path-resolving-less {
composes: apply-color from '@/composed.module.less';
composes: apply-color from '=/composed.module.less';
}
2 changes: 2 additions & 0 deletions playground/css/index.html
Expand Up @@ -32,6 +32,7 @@ <h1>CSS</h1>
contains alias
</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>Imported SASS string:</p>
<pre class="imported-sass"></pre>
<p class="sass-dep">
Expand All @@ -46,6 +47,7 @@ <h1>CSS</h1>
@import from Less: This should be darkslateblue and have bg image which url
contains alias
</p>
<p class="less-url-starts-with-variable">url starts with variable</p>
<p>Imported Less string:</p>
<pre class="imported-less"></pre>

Expand Down
2 changes: 1 addition & 1 deletion playground/css/less.less
@@ -1,4 +1,4 @@
@import '@/nested/nested';
@import '=/nested/nested';
@import './nested/css-in-less.less';

// Test data-uri calls with relative images.
Expand Down
2 changes: 1 addition & 1 deletion playground/css/linked.css
@@ -1,4 +1,4 @@
@import '@/linked-at-import.css';
@import '=/linked-at-import.css';

/* test postcss nesting */
.wrapper {
Expand Down
8 changes: 7 additions & 1 deletion playground/css/nested/_index.scss
Expand Up @@ -7,5 +7,11 @@

.sass-at-import-alias {
color: olive;
background: url(@/nested/icon.png) 10px no-repeat;
background: url(=/nested/icon.png) 10px no-repeat;
}

$var: '/ok.png';
.sass-url-starts-with-variable {
background: url($var);
background-position: center;
}
8 changes: 7 additions & 1 deletion playground/css/nested/nested.less
Expand Up @@ -5,5 +5,11 @@

.less-at-import-alias {
color: darkslateblue;
background: url(@/nested/icon.png) 10px no-repeat;
background: url(=/nested/icon.png) 10px no-repeat;
}

@var: '/ok.png';
.less-url-starts-with-variable {
background: url('@{var}');
background-position: center;
}
2 changes: 1 addition & 1 deletion playground/css/nested/nested.sss
Expand Up @@ -5,4 +5,4 @@

.sugarss-at-import-alias
color: darkslateblue
background: url(@/nested/icon.png) 10px no-repeat
background: url(=/nested/icon.png) 10px no-repeat
2 changes: 1 addition & 1 deletion playground/css/nested/nested.styl
Expand Up @@ -3,4 +3,4 @@

.stylus-import-alias
color darkslateblue
background url('@/nested/icon.png') 10px no-repeat
background url('=/nested/icon.png') 10px no-repeat
8 changes: 4 additions & 4 deletions playground/css/sass.scss
@@ -1,9 +1,9 @@
@import '@/nested'; // alias + custom index resolving -> /nested/_index.scss
@import '@/nested/partial'; // sass convention: omitting leading _ for partials
@import '=/nested'; // alias + custom index resolving -> /nested/_index.scss
@import '=/nested/partial'; // sass convention: omitting leading _ for partials
@import 'css-dep'; // package w/ sass entry points
@import 'virtual-dep'; // virtual file added through importer
@import '@/pkg-dep'; // package w/out sass field
@import '@/weapp.wxss'; // wxss file
@import '=/pkg-dep'; // package w/out sass field
@import '=/weapp.wxss'; // wxss file

.sass {
/* injected via vite.config.js */
Expand Down
2 changes: 1 addition & 1 deletion playground/css/sugarss.sss
@@ -1,4 +1,4 @@
@import '@/nested/nested.sss'
@import '=/nested/nested.sss'

.sugarss
color: blue
2 changes: 1 addition & 1 deletion playground/css/vite.config.js
Expand Up @@ -14,7 +14,7 @@ module.exports = {
},
resolve: {
alias: {
'@': __dirname,
'=': __dirname,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced this to test less's variable. (less's variable starts with @)

related: https://github.com/vitejs/vite/pull/10741/files#diff-2cfbd4f4d8c32727cd8e1a561cffbde0b384a3ce0789340440e144f9d64c10f6R1608

spacefolder: __dirname + '/folder with space',
'#alias': __dirname + '/aliased/foo.css',
'#alias-module': __dirname + '/aliased/bar.module.css'
Expand Down