Skip to content

feat: update wikimedia/less.php from 4.x to 5.x #4225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant