Skip to content

Conversation

Krinkle
Copy link

@Krinkle Krinkle commented Jun 10, 2025

Reviewers should focus on:

  • No (new) PHP warnings/errors in the logs.
  • Ensure the app still renders the same way visually in terms of CSS rules from your Less stylesheet files.

Necessity

  • Has the problem that is being solved here been clearly explained?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Commit message:

The only two relevent changes in Less.php 5.0 are:

  • Remove import_callback.
  • Change default math from "always" (strictMath: false) to "parens-division".

import_callback

The import_callback option was deprecated in 4.x, because it exposed a number of internal classes, all of which had to be used in order to work, including carefully obtaining and then returning the pathAndUri[1] value.

The SetImportDirs() method has been available for a long time already, and is instead given a simple string path (already resolved), and must return null|array{0:?string,1:?string}. Thus if it doesn't want to override, it can simply not return (implied null), which is equiv to [null,null] and the second URI value is optional, defaulting to the default.

Upstream change: https://gerrit.wikimedia.org/r/1010962
Docs: https://github.com/wikimedia/less.php/blob/v4.4.1/lib/Less/Parser.php#L592-L617

math

In Less.js 1.x-3.x the default was strictMath: false, which evaluates math eagerly, instead of only "strictly" between parenthesis. This had the downside of interpreting "/" slashes in newer CSS syntax as Less division, instead of as separator between values in those CSS features (aspect-ratio, border-radius, font).

As such, Less.js 4.x and Less.php 5.x, rename "strictMath" to "math", with a new third option "parens-division" as the default alongside "always" (aka strictMath: false) and "parens" (aka strictMath: true). This new option still eagerly evaluated math in most cases (plus, minus, multiply) but no longer for division. I don't know if or when Flarum wants to adopt this new default, but for now, to minimise disruption and unblock the Less.php upgrade, I suggest simply setting strictMath: false explicitly so that Less.php 5 behaves the same as before.

This decouples the trivial Less.php upgrade from the (potentially non-trivial, or unwanted) math migration.

Ref https://phabricator.wikimedia.org/T366445.
Ref https://github.com/wikimedia/less.php/blob/master/CHANGES.md.

Krinkle added 2 commits June 9, 2025 17:18
The only two relevent changes in Less.php 5.0 are: * Remove `import_callback`. * Change default math from "always" (strictMath: false) to "parens-division". The import_callback option was deprecated in 4.x, because it exposed a number of internal classes, all of which had to be used in order to work, including carefully obtaining and then returning the `pathAndUri[1]` value. Upstream change: https://gerrit.wikimedia.org/r/1010962 Docs: https://github.com/wikimedia/less.php/blob/v4.4.1/lib/Less/Parser.php#L592-L617 == import_callback == The SetImportDirs() method has been available for a long time already, and is instead given a simple string path (already resolved), and must return `null|array{0:?string,1:?string}`. Thus if it doesn't want to override, it can simply not return (implied null), which is equiv to `[null,null]` and the second URI value is optional, defaulting to the default. == math == In Less.js 1.x-3.x the default was `strictMath: false`, which evaluates math eagerly, instead of only "strictly" between parenthesis. This had the downside of interpreting "/" slashes in newer CSS syntax as Less division, instead of as separator between values in those CSS features (aspect-ratio, border-radius, font). As such, Less.js 4.x and Less.php 5.x, rename "strictMath" to "math", with a new third option "parens-division" as the default alongside "always" (aka `strictMath: false`) and "parens" (aka `strictMath: true`). This new option still eagerly evaluated math in most cases (plus, minus, multiply) but no longer for division. I don't know if or when Flarum wants to adopt this new default, but for now, to minimise disruption and unblock the Less.php upgrade, I suggest simply setting `strictMath: false` explicitly so that Less.php 5 behaves the same as before. This decouples the trivial Less.php upgrade from the (potentially non-trivial, or unwanted) math migration. Ref https://phabricator.wikimedia.org/T366445. Ref https://github.com/wikimedia/less.php/blob/master/CHANGES.md.
@Krinkle Krinkle requested a review from a team as a code owner June 10, 2025 00:24
@Krinkle Krinkle changed the title Less upgrade feat: update wikimedia/less.php from 4.x to 5.x Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant