modularscale / modularscale-sass

Modular scale calculator built into your Sass
http://www.modularscale.com
MIT License
1.98k stars 132 forks source link

Deprecation warnings after upgrading Sass when using `ms` Variable #171

Open joyheron opened 3 years ago

joyheron commented 3 years ago

I just upgraded my Sass version to 1.33.0, but now I have a lot of deprecated messages whenever I use the ms function within my code base. Since I use it in many different places in my code base, this occurs quite often. Is there any chance you could update your package so that these messages disappear?

DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($ms-return, $b)

More info and automated migrator: https://sass-lang.com/d/slash-div

   ╷
35 │       $ms-return: $ms-return / $b;
   │                   ^^^^^^^^^^^^^^^
   ╵
    node_modules/modularscale-sass/stylesheets/modularscale/_pow.scss 35:19       ms-pow()
    node_modules/modularscale-sass/stylesheets/modularscale/_function.scss 15:13  ms-function()
    node_modules/modularscale-sass/stylesheets/modularscale/_sugar.scss 6:11      ms()
    lib/styles/base/_spacing.scss 10:15                                           @import
    lib/styles/base/index.scss 3:9                                                @import
    components/05-atoms/colors/index.scss 1:9                                     root stylesheet
scottkellum commented 3 years ago

Thank you. I have not updated in a while and Sass functionality as well as CSS has come a long way over the years. It may be about time for a re-write to take advantage of the new Sass module system. My main focus has been another typography tool, Typetura, and it's been a while since I've done modular scale stuff.

spenno commented 3 years ago

@joyheron: The same thing happened to me yesterday. Until this is updated, I have copied the stylesheets/_modularscale.scss Sass file and the stylesheets/modularscale folder into a vendors folder in my local source files and imported that version of the Sass file instead of the one in node_modules.

Then I updated the math divisions in three of the Sass partials to use Sass 1.33's new maths.div function.

You can see the changes I made here: https://github.com/modularscale/modularscale-sass/compare/3.x...spenno:feature/math-division?expand=1

And a quick summary of the changes made:

_functions.scss line 29: $ms-base: math.div($ms-base, $ratio); _pow.scss line 35: $ms-return: math.div($ms-return, $b); _strip-units.scss line 6: @return (math.div($val, ($val - $val + 1)));

I would submit a pull request for this @scottkellum, but it would completely break for people still using LibSass. Although LibSass is now deprecated, I'm guessing you would prefer a more elegant transition than that :)

scottkellum commented 3 years ago

@spenno Breaking changes are what Semantic Versioning and dependency lists are for. I may go ahead and do a major version bump.

Now that exponents are now native to the Sass that will simplify a ton of code. Might as well do a big re-write.

Note that I’ll likely make some changes to ms-respond in the process. Changes there may be breaking with that mixin, but I’ll try to avoid it. There have been a lot of advancements here and plenty of better ways to implement responsive typography.

joyheron commented 3 years ago

Thank you for looking into this!

strarsis commented 3 years ago

Related to exponents in sass: https://github.com/modularscale/modularscale-sass/pull/113

scottkellum commented 3 years ago

Status update: The thing holding this up now is the responsive logic

strarsis commented 3 years ago

Does it make sense to prepare it for the upcoming CSS Container Queries?

scottkellum commented 3 years ago

@strarsis Yes, part of the reason why I want to update it to output Typetura styles instead of locks/clamp as an option. It would be easier with post-processing stylesheets as opposed to pre-processing as you have to be more explicit in pre-processing. Use cases like #148 are difficult to build a nice syntax around.

scottkellum commented 3 years ago

I got things working and the syntax is going to be something like this:

@include ms.step using ($respond) {
  foo {
    // This will be responsive
    bar: ms.step(2, $settings: $respond);

    // If for some reason you don’t want something to be responsive in these blocks, you can do this
    baz: ms.step(2)

    // Shorthand will work here! And you can mix responsive and non-responsive values.
    padding: ms.step(2, $settings: $respond) ms.step(1)

  }
}

I do not think I will support clamp() or locks. It will either be media queries, container queries, or Typetura.

Related to issue #148

scottkellum commented 3 years ago

New release you can test out here: https://github.com/modularscale/modularscale-sass/releases/tag/v4.0.0.rc1

robsonsobral commented 3 years ago

@scottkellum , hi!

I'm sorry, but I didn't get the expose of the ms.$settings map. Why not to use with() and make the settings private?

@use '../vendor/modularscale' with (
  $settings: (base: 1rem, ratio: 1.25),
);

@forward '../vendor/modularscale';

It can be "imported" once and forwarded already configured.

scottkellum commented 3 years ago

@robsonsobral thanks Rob, I will definitely look into this approach. I do like the ability to change the settings throughout your style sheet as needed, so I might want to make this work with that functionality.

robsonsobral commented 3 years ago

https://sass-lang.com/documentation/at-rules/forward#configuring-modules

scottkellum commented 3 years ago

Thanks for your feedback @robsonsobral, I drafted a new release here: https://github.com/modularscale/modularscale-sass/releases/tag/v4.0.0.rc2

robsonsobral commented 3 years ago

@scottkellum , I've been really busy, but I would like to do some tests before the release, if you don't mind.

scottkellum commented 3 years ago

@robsonsobral I understand but I am also really busy. I would love a PR here if anyone has the time and is interested in contributing.

If you do contribute please keep any configuration and dependencies to a minimum.

If no one can help I may release next week without tests.

scottkellum commented 3 years ago

To clarify, @ everyone reading this.

I am looking for help to write tests and ensure distribution channels are solid before release.

robsonsobral commented 3 years ago

This lib/addon/plugin/toolkit/whatever has been a constant part of my work for years. So, I gonna to give it back to the community.

gvjacob commented 2 years ago

Sharing a fairly low-effort workaround that works well for me. Install sass-migrator and put the following script in your package.json

"postinstall": "sass-migrator division node_modules/modularscale-sass/stylesheets/modularscale/*.scss"

On every npm install, the script will convert any / divisions to math.div in the plugin's output files. Some context to sass-migrator here

joyheron commented 2 years ago

@scottkellum Thank you for your work on this!

I have been testing out the new 4.x implementation, and for the most part it seems to work (I made an Issue for the only error that I have: https://github.com/modularscale/modularscale-sass/issues/176 )

One things that I noticed is that the migration from 3.x to 4.x is a bit tricky because a CSS rule like width: ms(2) (which needs to be replaced with ms.step(2)) isn't recognized as an error by the Sass compiler so the error isn't obvious at compile time (since it isn't valid CSS, the rule is ignored and the layout is just a bit (or a lot) off). But other than that, I was successful in migrating my code.

I am very happy to continue testing in the future.

heun01 commented 1 year ago

Since nothing has happened here for a long time, can we still expect an update?