Skip to content

Conversation

sathomas
Copy link

CSS file URLs may legitimately include commas. The recommended CSS for Google Web Fonts, for example, might look like:

@import url('//fonts.googleapis.com/css?family=Open+Sans:300italic,400italic,600italic,700italic,400,300,600,700&subset=latin,cyrillic-ext,latin-ext,greek-ext');

This PR allows cssImportIgnore to support regular expressions and an array of strings or regular expressions in addition to the current format of a single "comma separated list."

Note: Contributor License Agreement for stephen@sathomas.me is on file.

@jrburke
Copy link
Member

jrburke commented Apr 1, 2014

I think cssImportIgnore needs an overhaul in general: it should have just accepted an array in the build.js form and command line use should have fallen into the "needArray" conversion done in build.js. However, that is a larger, backwards incompatible change, so going to schedule this for 3.0.

In the meantime though, the import inlining should skip URLs that begin with // as those indicate network URLs, which are not supported in the optimizer. If you do find those are slipping through now, then I would want to do a more targeted fix for that in the 2.x series.

@sathomas
Copy link
Author

sathomas commented Apr 7, 2014

I wasn't aware that r.js had any special handling for import URLs beginning with // (and I still can't find the logic that does so), but I'm new to the app so please forgive the ignorance. I can confirm that setting

'cssImportIgnore': '//fonts.googleapis.com/css?family=Open+Sans:300italic,400italic,600italic,700italic,400,300,600,700&subset=latin,cyrillic-ext,latin-ext,greek-ext',

does ignore inlining for the import URL in the example. I thought that was just a quirk of the greedy regex matching algorithm, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants