Custom css URI functions are breaking

[vjeux]

Prettier 1.13.4
Playground link

--parser css

Input:

div {
  background: -fb-datauri(/intern/images/org/product/default-image.png);
  background: -fb-datauri(/intern/images/organization/product/default-image.png);
}

Output:

div {
  background: -fb-datauri(/ intern/images/org/product/default-image.png);
  background: -fb-datauri(
    / intern/images/organization/product/default-image.png
  );
}

This is not urgent but would be nice to find a way not to put a space after the first /. I remember that we are doing some custom things for urls, maybe we can whitelist everything that ends with url/uri as a heuristic?

[alexander-akait]

@vjeux because -fb-datauri is considered as not url function. But we can handle this as url.

[vjeux]

I know it's fb-specific but would be great if we could :) Maybe do a more general heuristic like:

name.endsWith('url') || name.endsWith('uri')

instead of

name === 'uri'

[alexander-akait]

Just add -fs-datauri here https://github.com/prettier/prettier/blob/master/src/language-css/utils.js#L144

[alexander-akait]

@vjeux can break many code using endWiths, better use list of this functions

[vjeux]

Sounds good, thanks for the pointer!

[lydell]

@vjeux If it's possible to quote the URLs you can use that as a workaround.

Prettier 1.13.4
Playground link

--parser css

Input:

div {
  background: -fb-datauri("/intern/images/org/product/default-image.png");
  background: -fb-datauri("/intern/images/organization/product/default-image.png");
}

Output:

div {
  background: -fb-datauri("/intern/images/org/product/default-image.png");
  background: -fb-datauri(
    "/intern/images/organization/product/default-image.png"
  );
}

[alexander-akait]

Same problem #2835, try to fix both problem tommorow

[vjeux]

I had some internal discussions and the current regex also doesn't support "" nor \n inside. Right now there's only a 100 occurrences so it's not a huge deal. There's a new infra to extract those pieces from CSS that's in the work that will support " but it's likely going to take some time to release.

I believe that we should find a way inside of prettier to not put a space after the / anyway, but in terms of priority it's pretty low and if we don't put any special handling inside of prettier we'll be fine, we can workaround on our end when it becomes an issue.

[dhruvdutt]

can break many code using endWiths, better use list of this functions

@evilebottnawi Can you please explain this by some example?

[alexander-akait]

@dhruvdutt

.a {
  background: my-custom-scss-less-function-url('first_arg_is_not_url_but_can_have spaces and other strange-characters-+-*/');
}

[dhruvdutt]

Gotcha! So, what's the potential solution? Should we have a static array of strings like "-fb-datauri", "uri", "url"?

[alexander-akait]

@dhruvdutt

1. Switch on postcss-value-parse (url function have own algorithm for parsing)
2. Use static array for url functions, but scss allow to redefine url function and it can break some edge cases like

@function url($arg) {
  return 'url';
}

a {
  background: url('same-other-strange-arg')
}

But some limitation for css is normal

[suchipi]

@vjeux still relevant? Should we pursue this or are you fine using prettier ignore comments and closing this issue?

[alexander-akait]

@suchipi no, we should fix it, it is not easy, but possible

[suchipi]

I was gonna fix it really quick but didn't want to waste my time if it's not a problem for facebook anymore- vjeux indicated that they were changing the infra some around this

[vjeux]

We could put everything with " and then allow for spaces around. But if it was fixed within prettier that would be great. It's not urgent though, it only affects 40 files that are barely touched.

[thorn0]

Fixed by #9966