-
Notifications
You must be signed in to change notification settings - Fork 65
fix: handle multiple url() #237
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
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "fix: handle multiple url()", | ||
| "packageName": "@griffel/babel-preset", | ||
| "email": "[email protected]", | ||
| "dependentChangeType": "patch" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "fix: handle multiple url()", | ||
| "packageName": "@griffel/webpack-extraction-plugin", | ||
| "email": "[email protected]", | ||
| "dependentChangeType": "patch" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { makeStyles } from '@griffel/react'; | ||
|
|
||
| import blank from './blank.jpg'; | ||
| import empty from './empty.jpg'; | ||
|
|
||
| export const useStyles = makeStyles({ | ||
| root: { backgroundImage: `url(${blank}), url(${empty})` }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import _asset2 from './empty.jpg'; | ||
| import _asset from './blank.jpg'; | ||
| import { __styles } from '@griffel/react'; | ||
| import blank from './blank.jpg'; | ||
| import empty from './empty.jpg'; | ||
| export const useStyles = __styles( | ||
| { | ||
| root: { | ||
| Bcmaq0h: 'fp00rh9', | ||
| }, | ||
| }, | ||
| { | ||
| d: [`.fp00rh9{background-image:url(${_asset}),url(${_asset2});}`], | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import { makeStyles } from '@griffel/react'; | ||
|
|
||
| export const useStyles = makeStyles({ | ||
| data: { backgroundImage: 'url(data:image/jpeg;base64,/9j/4AAQSkZJRgABAQEAYABgAAD/2Q==)' }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { __styles } from '@griffel/react'; | ||
| export const useStyles = __styles( | ||
| { | ||
| data: { | ||
| Bcmaq0h: 'fclgv40', | ||
| }, | ||
| }, | ||
| { | ||
| d: [`.fclgv40{background-image:url(data:image/jpeg;base64,/9j/4AAQSkZJRgABAQEAYABgAAD/2Q==);}`], | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export function isAssetUrl(value: string): boolean { | ||
| return !value.startsWith('data:'); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| import { GriffelStyle } from '@griffel/core'; | ||
| import { parseStringWithUrl } from './parseStringWithUrl'; | ||
| import { tokenize } from 'stylis'; | ||
|
|
||
| import { isAssetUrl } from './isAssetUrl'; | ||
|
|
||
| /** | ||
| * Linaria v3 emits relative paths for assets, we normalize these paths to be relative from the project root to be the | ||
|
|
@@ -22,16 +24,19 @@ export function normalizeStyleRule( | |
| return ruleValue; | ||
| } | ||
|
|
||
| const result = parseStringWithUrl(ruleValue); | ||
| // Quotes in URL are optional, so we can also normalize them | ||
| // https://www.w3.org/TR/CSS2/syndata.html#value-def-uri | ||
| const url = result.url.replace(/['|"](.+)['|"]/, '$1'); | ||
| return tokenize(ruleValue).reduce((result, token, index, array) => { | ||
|
Member
Author
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. Instead of a regex I am using |
||
| if (token === 'url') { | ||
| // Quotes in URL are optional, so we can also normalize them | ||
| // https://www.w3.org/TR/CSS2/syndata.html#value-def-uri | ||
| const url = array[index + 1].replace(/['|"](.+)['|"]/, '$1').slice(1, -1); | ||
|
|
||
| if (url.startsWith('data:')) { | ||
| return `${result.prefix}${url}${result.suffix}`; | ||
| } | ||
| if (isAssetUrl(url)) { | ||
| array[index + 1] = `(${normalizeAssetPath(path, projectRoot, filename, url)})`; | ||
| } | ||
| } | ||
|
|
||
| return `${result.prefix}${normalizeAssetPath(path, projectRoot, filename, url)}${result.suffix}`; | ||
| return result + token; | ||
| }, ''); | ||
| } | ||
|
|
||
| export function normalizeStyleRules( | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import { NodePath, traverse, types as t } from '@babel/core'; | ||
| import * as path from 'path'; | ||
| import { tokenize } from 'stylis'; | ||
|
|
||
| import { absolutePathToRelative } from './absolutePathToRelative'; | ||
| import { parseStringWithUrl } from './parseStringWithUrl'; | ||
| import { isAssetUrl } from './isAssetUrl'; | ||
|
|
||
| /** | ||
| * Replaces assets used in styles with imports and template literals. | ||
|
|
@@ -29,6 +30,35 @@ export function replaceAssetsWithImports( | |
| return assetIdentifiers.get(assetPath) as t.Identifier; | ||
| } | ||
|
|
||
| function buildTemplateLiteralFromValue(value: string): t.TemplateLiteral { | ||
| const tokens = tokenize(value); | ||
|
|
||
| const quasis: t.TemplateElement[] = []; | ||
|
Contributor
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. what are quasis ?
Member
Author
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. It's how Babel names string in template literals i.e. |
||
| const expressions: t.Identifier[] = []; | ||
|
|
||
| let acc = ''; | ||
|
|
||
| for (let i = 0, l = tokens.length; i < l; i++) { | ||
| acc += tokens[i]; | ||
|
|
||
| if (tokens[i] === 'url') { | ||
| const url = tokens[i + 1].slice(1, -1); | ||
|
|
||
| if (isAssetUrl(url)) { | ||
| quasis.push(t.templateElement({ raw: acc + '(' }, false)); | ||
| expressions.push(getAssetIdentifier(url)); | ||
|
|
||
| acc = ')'; | ||
| i++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| quasis.push(t.templateElement({ raw: acc }, true)); | ||
|
|
||
| return t.templateLiteral(quasis, expressions); | ||
| } | ||
|
|
||
| traverse( | ||
| pathToUpdate.node, | ||
| { | ||
|
|
@@ -39,15 +69,7 @@ export function replaceAssetsWithImports( | |
| return; | ||
| } | ||
|
|
||
| const result = parseStringWithUrl(value); | ||
| const assetIdentifier = getAssetIdentifier(result.url); | ||
|
|
||
| const templateLiteral = t.templateLiteral( | ||
| [t.templateElement({ raw: result.prefix }, false), t.templateElement({ raw: result.suffix }, true)], | ||
| [assetIdentifier], | ||
| ); | ||
|
|
||
| literalPath.replaceWith(templateLiteral); | ||
| literalPath.replaceWith(buildTemplateLiteralFromValue(value)); | ||
| }, | ||
| }, | ||
| programPath.scope, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,11 +35,6 @@ pluginTester({ | |
| // Fixtures | ||
| // | ||
| // | ||
| { | ||
| title: 'assets import handling', | ||
| fixture: path.resolve(fixturesDir, 'assets', 'code.ts'), | ||
| outputFixture: path.resolve(fixturesDir, 'assets', 'output.ts'), | ||
| }, | ||
|
Comment on lines
-38
to
-42
Member
Author
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. Moved into a separate group. |
||
| { | ||
| title: 'duplicated imports', | ||
| fixture: path.resolve(fixturesDir, 'duplicated-imports', 'code.ts'), | ||
|
|
@@ -59,6 +54,25 @@ pluginTester({ | |
| outputFixture: path.resolve(fixturesDir, 'non-existing-module-call', 'output.ts'), | ||
| }, | ||
|
|
||
| // Assets | ||
| // | ||
| // | ||
| { | ||
| title: 'assets: import handling', | ||
| fixture: path.resolve(fixturesDir, 'assets', 'code.ts'), | ||
| outputFixture: path.resolve(fixturesDir, 'assets', 'output.ts'), | ||
| }, | ||
| { | ||
| title: 'assets: multiple url()', | ||
| fixture: path.resolve(fixturesDir, 'assets-multiple-declarations', 'code.ts'), | ||
| outputFixture: path.resolve(fixturesDir, 'assets-multiple-declarations', 'output.ts'), | ||
| }, | ||
| { | ||
| title: 'assets: url() without imports', | ||
| fixture: path.resolve(fixturesDir, 'assets-urls', 'code.ts'), | ||
| outputFixture: path.resolve(fixturesDir, 'assets-urls', 'output.ts'), | ||
| }, | ||
|
|
||
| // Evaluation | ||
| // | ||
| // | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { __styles } from '@griffel/react'; | ||
| import _asset from './blank.jpg'; | ||
| import _asset2 from './empty.jpg'; | ||
|
|
||
| export const useStyles = __styles( | ||
| { | ||
| root: { | ||
| Bcmaq0h: 'fp00rh9', | ||
| }, | ||
| }, | ||
| { | ||
| d: [`.fp00rh9{background-image:url(${_asset}),url(${_asset2});}`], | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ["blank.jpg", "bundle.js", "empty.jpg", "griffel.css"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| .fp00rh9 { | ||
| background-image: url(blank.jpg), url(empty.jpg); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { __styles, __css } from '@griffel/react'; | ||
| export const useStyles = __css({ | ||
| root: { | ||
| Bcmaq0h: 'fp00rh9', | ||
| }, | ||
| }); | ||
|
|
||
| import 'griffel.css!=!../../../virtual-loader/index.js!../../../virtual-loader/griffel.css?style=.fp00rh9%7Bbackground-image%3Aurl(..%2F__fixtures__%2Fwebpack%2Fassets-multiple%2Fblank.jpg)%2Curl(..%2F__fixtures__%2Fwebpack%2Fassets-multiple%2Fempty.jpg)%3B%7D'; |

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.
I intentionally didn't cover this by tests directly as it will be changed/removed in #234.