scssphp
scssphp copied to clipboard
[RFC] Roadmap to 2.0
Based on the discussion in #201 and #193, I'd like to discuss a plan for a 2.0 release, allowing to respect semver even for package extending Scssphp today.
Until now, the refactoring done in #188 is backward compatible for most cases of people writing custom functions. The only breakages are:
- mutating a Number object is not allowed anymore (it would have broken lots of things before anyway, due to objects not being passed around by value and so mutating more things than expected, so this is rather a good thing)
- code needing to deal with the units of a Number, other than passing
$value[2]
to the constructor ofNumber
to create a new one, as the format changed. - code using
$number->units
, but a BC layer for that could even probably be implemented for reads with a magic property (I don't want to allow it for writes, as that would mutate the object) - code relying on
$number->dimension
being public (we could use a magic property for reads too).
#205 is making much more BC breaks in that class, and my upcoming work on values as value objects would of course be a BC break for custom functions as they would need to be rewritten. As that work would deserve a semver major release (there are some packages on Packagist which rely on the custom function APIs, though not sure they are maintained, and I don't know about any private ecosystem).
Based on these discussion, I would say that the only BC breaks we need for the custom functions API and custom variable API to be modernized are:
- the
Value
abstraction, as that will change the way custom functions are registered - making the prototype mandatory in
registerFunction
, so that our code can deal with all the processing of arguments and their error reporting - changing
setVariables
to take an argument beingarray<string, Value|string>
. This would preserve BC for cases passing a Scss to set the value (the reliable part of the current API) but break BC for the case of passing a PHP value (for lucky cases where it does not break by parsing that as scss successfully, if such case even exist). - changing
getVariables
to returnarray<string, Value>
(instead of the messy undocumented array it returns today) - making
Compiler
final to forbid extending it to register custom functions through theCompiler::lib*
mechanism (using->registerFunction
should be used for that) - tagging lots of public functions of the
Compiler
as@internal
or actually reducing their visibility so that they are not relied on from the outside - forbidding custom formatter implementation (so that the messy
OutputBlock
API becomes an implementation detail), see #162
Once these changes are done, any other refactoring to rewrite the library as object-oriented becomes internal-only and so does not require a semver-major anymore but can be done in semver-minor releases.
My proposal here would be to make a 1.4 release which perform the formatter deprecation, deprecates extending Compiler
and maybe adds the partial BC layer in Number
(if we consider it worth it), create a 1.x
branch to be able to make security releases for 1.x if necessary and make master
the dev branch for 2.x
in which we perform these refactorings (which is mostly the Value
refactoring as making the Compiler final and dropping setFormatter
will be no-brainers). We would release 2.0 when that refactoring is done, and continue the incremental rewrite towards spec compliance in further 2.x minor versions.
At that point, I suggest that 1.x receives only critical fixes, as trying to improve spec compliance on the old codebase in parallel of the new one would be a waste of resources. That's also the reason why I intentionally don't suggest bumping the PHP requirement in 2.0, based on the discussion in #202. This way, the upgrade should be a no-brainer for anyone not using custom functions (and still possible for projects using custom functions, with much cleaner code as a result).
What do you think about that plan @Cerdic @robocoder ?
Note that the exact plan for the 1.4 release vs the refactoring of values might be adjusted as such:
- create a 1.x branch immediately
- bump
master
to be 2.x-dev, allowing to merge #205 and additional refactorings - work on the necessary deprecations for 1.4 in the 1.x branch in parallel
- merge 1.x into master regularly when we do changes in 1.x
- turn 1.x into a maintenance branch once 2.0 is out.
However, I would expect that the time where we work in 2 branches stay short enough.
The 2.0 release could also be the release in which we make iconv
required (see #203), with a deprecation warning in 1.4 if you don't have it (hinting at the fact that it breaks spec compliance).
That's seems good to me as long as we keep the master branch running and spec compliant all the way long, which is exactly what you are proposing in #211
It's perfectly fine we don't plan to release until this big refactoring is done, but I also know that some plans you make for weeks can stay for years.
So if the refactoring is in standby at one point, we have to be able to release an "unfinished" - but still fully working - 2.0 version (unfinished in the meaning as not everything we want to be in it would be done)
Also I think for making things more undestandable we should rename the dart-sass port project https://github.com/scssphp/scssphp2 - could be scssphp-reloaded or whatever not relying on a version number.
I don't know how things will be in the future and maybe at one point we will decide incremental refactoring is too messy and a dead end and we should better work on this port, but however it would be helpfull to avoid users to be lost between the v2 of scssphp(1) and the v1 of scssphp2 imho
Also I think for making things more undestandable we should rename the dart-sass port project https://github.com/scssphp/scssphp2 - could be scssphp-reloaded or whatever not relying on a version number.
agreed (and done)
Sounds good to me. scssphp2
was just a placeholder name.
@stof Regarding to iconv
, Symfony has us covered in there, too. I was looking through our dependencies and found polyfill library for that, too.
@mahagr yeah, that could be used for people who cannot even have ext-iconv
. But note that this package is huge. and performance will suffer compared to the extension.