Skip to content

Commit

Permalink
Fix absolute path in custom CSS url() calls (#9966)
Browse files Browse the repository at this point in the history
At Facebook we have some custom pre-processor `-fb-url(/abs/path/)` that gets messed up as `-fb-url(/ abs/path)`  with the current version of prettier.

The parser has hardcoded logic to only parse as path things that use `url()` but not `-fb-url()` and it parses it as `division /` and `abs/path/`. Because we put a space after division, the bug is introduced.

I don't really know the impact of changing the parser, but on the printer side, it is straightforward to not add a space after `/` if it's the first element in a group. I can't think of any reason why you could write `(/ something` in CSS so I think it's a safe change.
  • Loading branch information
vjeux committed Dec 28, 2020
1 parent 60b8ad0 commit eec3e82
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 0 deletions.
14 changes: 14 additions & 0 deletions changelog_unreleased/css/9966.md
@@ -0,0 +1,14 @@
#### Fix absolute path in custom CSS -custom-url() calls (#9966 by @vjeux)

The CSS parser is parsing this as `["division", "absolute/path"]` instead of a single `"/absolute/path"` token unless you are in a `url()` call. Because we put space after division, it results in an incorrect path. The fix was to avoid printing a space if a division is the first token of a call, which hopefully should be safe.

<!-- prettier-ignore -->
```css
-custom-url(/absolute/path)

/* Prettier stable */
-custom-url(/ absolute/path)

/* Prettier master */
-custom-url(/absolute/path)
```
7 changes: 7 additions & 0 deletions src/language-css/printer-postcss.js
Expand Up @@ -675,6 +675,13 @@ function genericPrint(path, options, print) {
continue;
}

// absolute paths are only parsed as one token if they are part of url(/abs/path) call
// but if you have custom -fb-url(/abs/path/) then it is parsed as "division /" and rest
// of the path. We don't want to put a space after that first division in this case.
if (!iPrevNode && isDivisionNode(iNode)) {
continue;
}

// Print spaces before and after addition and subtraction math operators as is in `calc` function
// due to the fact that it is not valid syntax
// (i.e. `calc(1px+1px)`, `calc(1px+ 1px)`, `calc(1px +1px)`, `calc(1px + 1px)`)
Expand Down
21 changes: 21 additions & 0 deletions tests/css/url/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`url.css format 1`] = `
====================================options=====================================
parsers: ["css"]
printWidth: 80
| printWidth
=====================================input======================================
div {
background: url(/images/bg.png);
background: -fb-url(/images/bg.png);
}
=====================================output=====================================
div {
background: url(/images/bg.png);
background: -fb-url(/images/bg.png);
}
================================================================================
`;
1 change: 1 addition & 0 deletions tests/css/url/jsfmt.spec.js
@@ -0,0 +1 @@
run_spec(__dirname, ["css"]);
4 changes: 4 additions & 0 deletions tests/css/url/url.css
@@ -0,0 +1,4 @@
div {
background: url(/images/bg.png);
background: -fb-url(/images/bg.png);
}

0 comments on commit eec3e82

Please sign in to comment.