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

Add !default suffix to all Sass variables #410

Merged
merged 1 commit into from
Jul 12, 2016

Conversation

vdh
Copy link
Contributor

@vdh vdh commented Jul 7, 2016

This change to lessToSass.js adds !default after the end of all variables in Sass files.

One thing that I really like about Sass is variable defaults, so that variables can be defined once at the beginning before importing dependencies instead of having to override variables after the fact. LESS instead has "lazy loading" of variables which makes it technically unneccessary to have defaults, but also causes a lot of confusion about the way LESS vs Sass handle their variables. LESS variables are a lot like Javascript hoisting, which tends to rub me the wrong way. Call me old fashioned, but I just prefer a nice simple top-down variable definition.

For example instead of needing to do this (via sass-loader in Webpack, hence the use of ~):

@import '~react-widgets/lib/scss/variables';
@import '~react-widgets/lib/scss/bootstrap-theme';

@import 'react-widgets-config'; // Custom variable overrides

@import '~react-widgets/lib/scss/mixins';
@import '~react-widgets/lib/scss/normalize';
@import '~react-widgets/lib/scss/icons';

@import '~react-widgets/lib/scss/core';
@import '~react-widgets/lib/scss/popup';
@import '~react-widgets/lib/scss/datepicker';
@import '~react-widgets/lib/scss/selectlist';
@import '~react-widgets/lib/scss/multiselect';

It can be replaced with a much more compact import:

@import 'react-widgets-config';
@import '~react-widgets/lib/scss/react-widgets';

Similar to how someone might do this with LESS and its lazy loading (via less-loader):

@import '~react-widgets/lib/less/react-widgets';
@import 'react-widgets-config';

@vdh vdh force-pushed the sass-variable-defaults branch from 6e37370 to 1c8d397 Compare July 12, 2016 03:24
@vdh
Copy link
Contributor Author

vdh commented Jul 12, 2016

Rebased to the latest master branch, and removed the changes to lib/* files.

@jquense jquense merged commit db6cd67 into jquense:master Jul 12, 2016
@vdh vdh deleted the sass-variable-defaults branch July 12, 2016 12:25
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.

2 participants