Discuss: More semver major releases?

[chriseppstein]

node-sass@4.9.0 had some backwards incompatible changes due to the libSass update which fixed nonstandard behavior in libSass compared to the reference implementation. Specifically, named colors stopped being able look up in a map by a color if the name of the color was in the map as a string. (Note: I suspect this change in libsass was the culprit.)

It's not uncommon for code to rely on such aspects of non-standard implementations, even when developers know what the right way to write the code might be, it can be easy to forget to type "black" instead of black and for a code review to miss this.

No qualms with this fix. But I think we should discuss whether node-sass should be more careful in terms of semver. Because ^ dependencies are the npm default, we picked up this upgrade automatically and it caused failed builds and a fair bit of churn at work today for our engineers.

Thoughts?

[stefanpenner]

and it caused failed builds and a fair bit of churn at work today for our engineers.

The solution for us has been to blacklist node-sass >= 4.9.0, as the amount of work to backport fixes to all previously affected apps/modules is extremely high.

Although the libsass fix is a bugfix in a strict sense, it has been a feature for node-sass for quite some time. Because of this, I would be inclined to recommend this libsass patch bugfix, be a major bump for node-sass.

The cost of this being a major bump for node-sass is pretty low, and it mitigates a good amount of "pain".

[xzyfer]

I want to start by apologizing for the pain caused.

More frequently bumping the major, specifically for LibSass releases has been considered before. There are pros and cons on either side, but it's something were only to re-evaluating.

A lot of effort is put into avoiding releasing breaking changes into the node-sass ecosystem when bumping LibSass. So much so we often maintain separate LibSass branches that opt out of risky changes until we're in a place to bump the major.

The issues @chriseppstein linked to (sass/libsass#2594) is one of those such changes. It landing on LibSass master forced us to create a separate branch (absent of that change) for the node-sass ecosystem.

Unfortunately I dropped the ball when backporting what seemed like an innocuous bug fix Fix parsing of colors (remove dynamic cast on eval) into the stable node-sass branch.

This was a communication break down within the LibSass team, that resulted in me misjudging the scope of a seemingly minor change.

---

As for bumping the major for each LibSass bump. Our primary concern is the churn this would cause on the eco-system.

We have taken some steps since v4 for LibSass bumps

On the LibSass side

• smaller, more frequent releases
• a more risk adverse release line for node-sass
• testing potential releases against a range of open source projects

On the node-sass side

• minor version bumps for libSass bumps
• beta releases for libSass bumps

Unfortunately the reach of the @nodesass, and my own hasn't been sufficient to generate enough uptake to catch these kinds of issue.

---

Next steps. I am OK with releasing a node-sass@v4.10 with this specific change removed (it will re-land in v5).

I would also like to discuss a process for getting y'alls codebases involved in more structured beta testing program to avoid these kinds of issues in the future.

[stefanpenner]

I want to start by apologizing for the pain caused.
Unfortunately I dropped the ball when backporting what seemed like an innocuous bug fix Fix parsing of colors (remove dynamic cast on eval) into the stable node-sass branch.

Not a problem, IMHO this feels like it falls into the realm of totally reasonable issues. I would have also assumed "innocuous bug fix". Thanks for being super responsive :)

Next steps. I am OK with releasing a node-sass@v4.10 with this specific change removed (it will re-land in v5).

This sounds perfect!

[chriseppstein]

As for bumping the major for each LibSass bump.

I don't think this is required. As long as a reasonable effort is made to note when fixes may have backwards incompatibilities with non-standard syntax -- I think it's impossible to be 100% accurate on this.

I would also like to discuss a process for getting y'alls codebases involved in more structured beta testing program to avoid these kinds of issues in the future.

@xzyfer We're interested in participating on this. Send me an email?

[xzyfer]

Great. I'm hoping between airports for the next couple days but will email you when I'm settled. Can you please DM the appropriate email address?

…

On Sat., 5 May 2018, 5:54 am Chris Eppstein, ***@***.***> wrote: As for bumping the major for each LibSass bump. I don't think this is required. As long as a reasonable effort is made to note when fixes may have backwards incompatibilities with non-standard syntax -- I think it's impossible to be 100% accurate on this. I would also like to discuss a process for getting y'alls codebases involved in more structured beta testing program to avoid these kinds of issues in the future. @xzyfer <https://github.com/xzyfer> We're interested in participating on this. Send me an email? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2364 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWCMA_9RhmBNcNk-jpFGo8u-NJ8nrks5tvM4EgaJpZM4TwNc5> .

[xzyfer]

Apologies for the radio silence, I have only just returned from vacation.

I'm growing comfortable rolling this back given:

• the recent disruption caused by Including .css files with @import is non-standard behaviour which will be removed in future versions of LibSass. #2362
• the amount of time this has been out in wild
• the absence of noise regarding this change

It's rare we're able to release correction (spec wise) such as this with such relatively little disruption. I acknowledge that rolling your code base forward is not a small job but I'm inclined to not rock the boat.