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

Breaking changes: move functional styles inline #1167

Open
mihkeleidast opened this issue Oct 18, 2021 · 3 comments
Open

Breaking changes: move functional styles inline #1167

mihkeleidast opened this issue Oct 18, 2021 · 3 comments
Labels
Feature Feature requests/suggestions

Comments

@mihkeleidast
Copy link

First, a big thank you for this module. It's really helped out a lot over the years.

An issue rose from the last minor update:

Fixed: Compositing issues in Safari (note that this updates the stylesheet!) (#987, #998, #1029, #1154);

This changed the stylesheet, which is a breaking change - any libraries wrapping nouislider with their own stylesheet, but installing it via a transitive dependency, were broken. So if my lib is using ^15.4.0 and my lib is installed in a project, npm will install 15.5.0 in the project, misaligning the styles and breaking the project.

So, yeah - does noUiSlider follow semver? Should any libs that use caret to mark dependencies or something else (~15.4.0 for example)? Or was this change just not considered breaking, though it came out to be?

@leongersen
Copy link
Owner

Thanks for your kind words, I'm happy to hear the library has helped you.

Yes, noUiSlider follows SemVer. The problem here is the stylesheet: practically every change there is potentially breaking for everyone with overriding styles. When importing the compiled script and stylesheet without overrides, there are no breaking changes within major versions.

There's basically two parts to the stylesheet: the functional styling and the visual styling. Ideally, the functional part (which had a change in 15.5.0) would be considered @internal and overriding it would not be covered by BC-compatibility. There's no good way to annotate this, and no way to validate/enforce this while using an external style sheet.

Another approach would be to move the functional part of the stylesheet into the JavaScript. This would be a bit of an endeavor and a breaking change in itself, but it might be worth doing to avoid issues like this. What do you think? I'm very open to feedback in this area.

@mihkeleidast
Copy link
Author

Yeah, totally understand that this is complicated and not very straightforward.

Regarding the specific circumstance, this had a change in both script and style, which can break more than some other changes. That's why I would have considered this a breaking change on it's own.

I think the specific width style could be moved to be inline on the element (the transform that also uses the multiplies already is), so if the multiplier should change further, the inline style would always trump whatever the user has provided in their CSS file. Not a fan of inline styles, but when working with these kinds of plugins, they're basically unavoidable. So yeah, I guess I would be for pulling the functional styles into JS in this case.

@leongersen leongersen added the Feature Feature requests/suggestions label Oct 18, 2021
@leongersen leongersen added this to the Wishlist milestone Oct 18, 2021
@leongersen leongersen changed the title Does noUiSlider follow semver? Breaking changes: move functional styles inline Oct 18, 2021
@Aareksio
Copy link

Hey! I stumbled upon the same issue updating dependencies. Since the stylesheet provided by nouislider includes not only the functional part, but also actual styles, we decided to extract the functional part ourselves. This led to display issues (customer-reported):

image

I am personally against moving the styles to JavaScript, but perhaps you could consider allowing import of only the functional part? This way projects which want full control over the style without worrying about overrides could import just the functional part you treat as not necessarily backwards-compatible.

- import 'nouislider/dist/nouislider.css';
+ import 'nouislider/dist/nouislider.functional.css';

This change would be non breaking, cheap to implement and opt-in, as nouislider.css would still be shipped as it currently is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Feature requests/suggestions
Projects
None yet
Development

No branches or pull requests

3 participants