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

[email protected] breaks optional parameters for functions? #2059

Closed
merwan7 opened this issue Aug 10, 2023 · 14 comments
Closed

[email protected] breaks optional parameters for functions? #2059

merwan7 opened this issue Aug 10, 2023 · 14 comments

Comments

@merwan7
Copy link

merwan7 commented Aug 10, 2023

Hello, I recently ran into this and thought I'd report it in case someone else runs into it.

We have a function we've been using such as this one:

/**
 * Pixels to rem
 */
@function rem($pixels, $context: 16) {
  @return #{math.div($pixels, $context)}rem;
}

When we upgraded to [email protected] using this function like this:

.some-selector {
  padding: rem(24);
}

Throws this error:

[sass] 2 arguments required, but only 1 was passed.
   ╷
11 │   padding: rem(24);
   │            ^^^^^^^
   ╵

This works fine in 1.64.x

You can see this in action here: https://stackblitz.com/edit/vitejs-vite-xuicb1?file=src%2FApp.scss

I checked the changelog and didn't see anything related to this. I haven't had time to look into what may have caused this, but I'll take a look in a few to see if I can identify what happened.

@merwan7 merwan7 changed the title sass 1.65 breaks optional parameters for functions? [email protected] breaks optional parameters for functions? Aug 10, 2023
@Josep-Lancharro-Indra
Copy link

Same here, we've returned to 1.64.2 for the moment

@juvenrui
Copy link

ugrade v1.65.1, and output: "Error: 2 arguments required, but only 1 was passed."

@UncleKhab
Copy link

UncleKhab commented Aug 10, 2023

The upgrade to v1.65.1 throws "Error: 2 arguments required, but only 1 was passed.". We currently locked the version to v1.64.2 to fix the issue.

@juvenrui
Copy link

The upgrade to v1.65.1 throws "Error: 2 arguments required, but only 1 was passed.". We currently locked the version to v1.64.2 to fix the issue.

homebrew can not reinstall [email protected] 🥵

@ntkme
Copy link
Contributor

ntkme commented Aug 10, 2023

CHANGELOG of 1.65.0:

All functions defined in CSS Values and Units 4 are now parsed as calculation objects: round(), mod(), rem(), sin(), cos(), tan(), asin(), acos(), atan(), atan2(), pow(), sqrt(), hypot(), log(), exp(), abs(), and sign().

It looks like because this is implemented as css syntax parser, it will no longer be parsed as a sass function if you have one with the same name. Rename sass function rem to something else and it should work.

@merwan7
Copy link
Author

merwan7 commented Aug 10, 2023

CHANGELOG of 1.65.0:

All functions defined in CSS Values and Units 4 are now parsed as calculation objects: round(), mod(), rem(), sin(), cos(), tan(), asin(), acos(), atan(), atan2(), pow(), sqrt(), hypot(), log(), exp(), abs(), and sign().

It looks like because this is implemented as css syntax parser, it will no longer be parsed as a sass function if you have one with the same name. Rename sass function rem to something else and it should work.

Thanks, I totally missed that rem was in that list. Will close this issue since this explains it!

@sqal
Copy link

sqal commented Aug 11, 2023

@ntkme but why can't I use the name e.g. _rem? I still get the same error 2 arguments required, but only 1 was passed.

@function _rem($value) {
  @return (math.div($value, 16px)) * 1rem
}

.test {
  font-size: _rem(32px);
}

@ntkme
Copy link
Contributor

ntkme commented Aug 11, 2023

@sqal The following works normally for me. Maybe you have other code that is triggering the error?

@use "sass:math";

@function _rem($value) {
  @return (math.div($value, 16px)) * 1rem;
}

.test {
  font-size: _rem(32px);
}

It outputs:

.test {
  font-size: 2rem;
}

@sqal
Copy link

sqal commented Aug 11, 2023

@ntkme decided to rename to toRem because _rem doesn't quite work. I am not sure if it's a bug or I am simply missing something from the docs that function names cannot start with an underscore? Here's an example that doesn't work

// mixins.scss

@use "sass:math";

// the name can be anything as long as it starts with underscore
@function _rem($value) {
  @return (math.div($value, 16px)) * 1rem
}

input.scss

@use "./mixins.scss" as *; // (!) imported from another file

$size: _rem(16px);

.test {
  width: $size * 2;
}

result:

Error: Undefined operation "_rem(16px) * 2".
  ╷
6 │   width: $size * 2;
  │          ^^^^^^^^^
  ╵

[email protected]

@ntkme
Copy link
Contributor

ntkme commented Aug 11, 2023

@sqal Functions prefixed with underscore are considered "private members": https://sass-lang.com/documentation/at-rules/use/#private-members

As a stylesheet author, you may not want all the members you define to be available outside your stylesheet. Sass makes it easy to define a private member by starting its name with either - or _. These members will work just like normal within the stylesheet that defines them, but they won’t be part of a module’s public API. That means stylesheets that load your module can’t see them!

@Goodwine
Copy link
Member

An alternative is not loading the mixin without a namespace (i.e. not using as *), then you can use mixins.rem(...)

@marcovtwout
Copy link

Should rem() even be part of a stable release of the dart-sass engine? According to mdn it is experiemental technology so the API might not even be stable?: https://developer.mozilla.org/en-US/docs/Web/CSS/rem

Experimental: This is an experimental technology
Check the Browser compatibility table carefully before using this in production.

@nex3
Copy link
Contributor

nex3 commented Aug 17, 2023

I've reopened sass/sass#3504 to track this.

@televators
Copy link

televators commented May 9, 2024

I've been using a rem function mixin basically exactly like mewan7's for YEARS and just today ran into this and was getting very confused and frustrated. I'm going to try rolling back to v1.64.x I guess but that's not going to be sustainable in the long term.

EDIT:
I just realized I was doing a silly that happened to present exactly the same as this issue. Using v1.77.0 currently in a very simple static project. I realized that I wasn't actually loading my rem() mixin in the partial in which I ran into the issue in the first place. Once I added @use 'mixin' as *;, the issue went away. I then actually checked @nex3 's comment on sass/sass#3504 about reverting until Dart Sass v3.0 and realized it couldn't be that breaking change affecting me on v1.77.0.

Basically, if you were coding while sleepy and forgot to actually @use your rem() mixin, it will give you the exact same appearance as this issue from v1.65.0 since Sass will assume you're using that experimental rem() calc function but the actual issue with properly loaded functions isn't actually present. Leaving this for another goofball like me just in case.

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

10 participants