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

extend and fix data url imports #534

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ function AtImport(options) {
state,
[],
[],
"",
postcss
)

Expand Down
20 changes: 16 additions & 4 deletions lib/data-url.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
"use strict"

const dataURLRegexp = /^data:text\/css;base64,/i
const anyDataURLRegexp = /^data:text\/css(?:;(base64|plain))?,/i
const base64DataURLRegexp = /^data:text\/css;base64,/i
const plainDataURLRegexp = /^data:text\/css;plain,/i

function isValid(url) {
return dataURLRegexp.test(url)
return anyDataURLRegexp.test(url)
}

function contents(url) {
// "data:text/css;base64,".length === 21
return Buffer.from(url.slice(21), "base64").toString()
if (base64DataURLRegexp.test(url)) {
// "data:text/css;base64,".length === 21
return Buffer.from(url.slice(21), "base64").toString()
}

if (plainDataURLRegexp.test(url)) {
// "data:text/css;plain,".length === 20
return decodeURIComponent(url.slice(20))
}

// "data:text/css,".length === 14
return decodeURIComponent(url.slice(14))
}

module.exports = {
Expand Down
19 changes: 12 additions & 7 deletions lib/parse-statements.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ function split(params, start) {
return list
}

module.exports = function parseStatements(result, styles) {
module.exports = function parseStatements(result, styles, from) {
const statements = []
let nodes = []

styles.each(node => {
let stmt
if (node.type === "atrule") {
if (node.name === "import") stmt = parseImport(result, node)
else if (node.name === "media") stmt = parseMedia(result, node)
else if (node.name === "charset") stmt = parseCharset(result, node)
if (node.name === "import") stmt = parseImport(result, node, from)
else if (node.name === "media") stmt = parseMedia(result, node, from)
else if (node.name === "charset") stmt = parseCharset(result, node, from)
}

if (stmt) {
Expand All @@ -39,6 +39,7 @@ module.exports = function parseStatements(result, styles) {
nodes,
media: [],
layer: [],
from,
})
nodes = []
}
Expand All @@ -52,23 +53,25 @@ module.exports = function parseStatements(result, styles) {
nodes,
media: [],
layer: [],
from,
})
}

return statements
}

function parseMedia(result, atRule) {
function parseMedia(result, atRule, from) {
const params = valueParser(atRule.params).nodes
return {
type: "media",
node: atRule,
media: split(params, 0),
layer: [],
from,
}
}

function parseCharset(result, atRule) {
function parseCharset(result, atRule, from) {
if (atRule.prev()) {
return result.warn("@charset must precede all other statements", {
node: atRule,
Expand All @@ -79,10 +82,11 @@ function parseCharset(result, atRule) {
node: atRule,
media: [],
layer: [],
from,
}
}

function parseImport(result, atRule) {
function parseImport(result, atRule, from) {
let prev = atRule.prev()
if (prev) {
do {
Expand Down Expand Up @@ -119,6 +123,7 @@ function parseImport(result, atRule) {
media: [],
layer: [],
supports: [],
from,
}

for (let i = 0; i < params.length; i++) {
Expand Down
18 changes: 16 additions & 2 deletions lib/parse-styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ async function parseStyles(
state,
media,
layer,
from,
postcss
) {
const statements = parseStatements(result, styles)
const statements = parseStatements(result, styles, from)

for (const stmt of statements) {
stmt.media = joinMedia(media, stmt.media || [])
Expand Down Expand Up @@ -93,6 +94,10 @@ async function resolveImportId(result, stmt, options, state, postcss) {
return
}

if (dataURL.isValid(stmt.from)) {
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps leave a comment on what we're doing here, and why? Won't be obvious to our future selves.


const atRule = stmt.node
let sourceFile
if (atRule.source?.input?.file) {
Expand Down Expand Up @@ -201,7 +206,16 @@ async function loadImportContent(
}

// recursion: import @import from imported file
return parseStyles(result, styles, options, state, media, layer, postcss)
return parseStyles(
result,
styles,
options,
state,
media,
layer,
filename,
postcss
)
}

module.exports = parseStyles
4 changes: 4 additions & 0 deletions test/fixtures/data-url.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@
/* Mixed imports: */
@import url(data:text/css;base64,QGltcG9ydCB1cmwoZm9vLmNzcyk7CgpwIHsKICBjb2xvcjogYmx1ZTsKfQo=);
@import url(data-url.css);

/* url encoded: */
@import url(data:text/css;plain,bar%20%7B%20color%3A%20green%20%7D);
@import url(data:text/css,bar%20%7B%20color%3A%20pink%20%7D);
26 changes: 4 additions & 22 deletions test/fixtures/data-url.expected.css
Original file line number Diff line number Diff line change
@@ -1,24 +1,6 @@
p { color: green; }
@import url(foo.css);p { color: green; }p { color: blue; }@media (min-width: 320px){@layer foo{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If a data url eventually unfurls into a regular import with a relative path it should stop and must not inline the relative import.

Each import is resolved relatively to it's parent and when the parent is a data url there isn't any domain or base url to resolve a url against.

I think usage of data urls was already very niche and that it is extremely unlikely that anyone depends on the old behavior.

p { color: green; } } }@media (min-width: 320px){@layer foo{

p { color: blue; }

@media (min-width: 320px) {

@layer foo {
p { color: green; } } }

@media (min-width: 320px) {

@layer foo {

p { color: blue; } } }

/* Mixed imports: */

foo{}

p {
p { color: blue; } } }/* Mixed imports: */p {
color: blue;
}

p { color: pink; }
}p { color: pink; }/* url encoded: */bar { color: green }bar { color: pink }