Skip to content
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

Reconsider asterisk-versioning of @csstools/normalize.css and sanitize.css #63

Closed
bmuenzenmeyer opened this issue Sep 21, 2021 · 4 comments

Comments

@bmuenzenmeyer
Copy link

broken out of #58 (comment) and #58 (comment)

Context

[email protected] introduced support for normalize and sanitze.css via #43.

This generous version range broke v10 and v9 ecosystems when sanitize.css removed a file that postcss-normalize imported.

#60 resolved the issue for v10, but this is not addressing the root cause, only fixing the problem until it breaks again. The changelog even alludes to the uncomfortable position we are in.

Question

Are you in theory amenable to work to remove asterisk-based versioning for direct dependencies, whether done yourself or contributed from the community? Previous comments linked above in #58 seem to indicate yes.

@jonathantneal
Copy link
Collaborator

I think pinning the version is reasonable. We would need to coordinate releases of sanitize.css, normalize.css, and postcss-normalize. But since I run all of them, the coordination is more logistical than organizational.

@wschnicke
Copy link

I'm not clear on why you would need more coordination with a pinned version. The entire purpose of pinning to a specific version (or range of versions) is to prevent issues in one package from impacting another.

Currently, the package seems to be broken due to the latest version of sanitize.css missing a file that is depended upon by postcss-normalize. Had the version been pinned in postcss-normalize, this would have been avoided.

@bmuenzenmeyer
Copy link
Author

bmuenzenmeyer commented Oct 1, 2021

I tried breaking this down. Looking for critical feedback on the below:

What Happened, When

[email protected] was released with proper semver September 14th, 2021.

At the time of it's release, the following versions of postcss-normalize broke:

you can verify these commands with npm view postcss-normalize@<VERSION> dependencies

Steps Taken To Resolve So Far

Risks That Still Remain

While all of the above was happening, normalize.css also released breaking changes September 15, 2021.

From the changelog:

- Changed: normalize.css no longer contributes specificity.
- Changed: normalize.css fixes now respect `writing-mode`.
- Removed: Fixes for older browsers, and all fixes for IE.
- Fixed: `table` uses correct `border-color: currentColor` normalization.

This is fine in and of itself, but not when it is also * asterisked within postcss-normalize.

We don't know the impact of this auto-upgrade through breaking changes, it didn't break as loudly as sanitize.css. Teams may have noticed the browser changes and taken steps to mitigate them.

Current State

  • v9/postcss7 ecosystem broken
    • regenerating lockfiles, over time, surface this issue to more and more users
  • v10/postcss8 and v9/postcss7 ecosystems still vulnerable
  • both versions affected by normalize.css breaking changes

Analysis

We generally cannot make many assumptions about what users in the wild have downloaded when resolving postcss-normalize ➡️ santize.css@* or postcss-normalize ➡️ normalize.css@*.

Users experiencing issues on v9 likely are using the resolutions workaround, so we don't have to sweat too much. v10 users are unblocked but still vulnerable. All users are impacts by breaking changes to normalize.css.

Pinning to an earlier version is problematic because you cannot undo a semver violation with another one. Why? Teams might have mitigated the changes from an auto-upgrade, only for a good-natured patch fix changing it back. Two wrongs don't make a right with semver.

Proposed Plan

Given the timeframes involved and the likely need to support both postcss7 and postcss8 ecosystem, I'd suggest the following:

v9/postcss7

Revert the v9 ecosystem to a state pre-September 14th. Not a lot of time has passed, and perhaps those still on v9/postcss7 have not yet re-generated lockfiles.

v10/postcss8

Deprecate all of v10 as unstable. We simply cannot know how this is playing out, or will continue to play out with *-based versioning. For example, users on v10 may have sanitize.css@12 or sanitize.css@13, depending on when they generated a lockfile. Pinning to one or the other breaks one population in order to stabilize the other. The good news is postcss8 is likely less-adopted for now.

v11/postcss8

SEMVER Major out of the *-based dependencies.

What About peerDependencies?

Someone in a long-ago issue asked if peerDependencies would better solve this problem. At this time I don't think so, especially on the advent on npm7 changing the algorithm. I guess it comes down to whether or not you'd expect people to independently be installing versions of your dependencies. Considering the ecosystem here, I'd suspect no, but am only one opinion.

@romainmenke
Copy link
Member

Seems fair.

I've released version 13 with pinned versions.

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

No branches or pull requests

4 participants