I think it was a design flaw that we went ahead with implementing php-land functions at all. It was a questionable experiment, but we know better now and I think we should admit that and phase it out ASAP.
This brings three main problems:
- Decreased compatibility; (can't easily compile it with lessjs without porting the functions over and keeping in sync).
- Extra maintenance; (when we switch lessphp implementations or upgrade, these will need updating).
- Separation of concerns and flow control; (embedding is supposed to be part of CSSMin and run *after* CSSJanus; doing it in LESS before it even turns to CSS disrupts this).
Problem #3 causes bugs like bug 66091 (T68091), where in Less files certain things don't get flipped properly by CSSJanus because it jumps the gun on mapping things to files on disk and reading them.
I think we should deprecate the less embed() function and instead ensure the /* @embed */. This provides many benefits:
- Reduced duplication.
- Less output is easier to debug with (no embedded data uris[1]).
- No maintenance overhead (and bugs like bug 66091) by having to keep its behaviour in sync with non-LESS files with regards to CSSJanus flipping.
- Keeps minification restricted to CSSMin (which runs anyway, the only way it makes sense to start minifying or reading files from disk earlier is if we somehow phase out CSSMin entirely, which would be an interesting adventure of its own)
Version: 1.24rc
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=66091 T68091: ResourceLoaderLESSFunctions::embed does not support CSSJanus flipping for RTL, affecting .background-image-svg() and others
https://bugzilla.wikimedia.org/show_bug.cgi?id=54673 T56673: LESS compiler should preserve the position of CSSMin / CSSJanus annotations