Skip to content
This repository has been archived by the owner on Aug 5, 2020. It is now read-only.

Fix Sass deprecation warning re: reverse-gradient function. #46

Closed
wants to merge 1 commit into from

Conversation

ry5n
Copy link
Contributor

@ry5n ry5n commented May 24, 2015

Since Sass 3.4.10 (January), passing anything other than a non-string value to the unquote function is deprecated. This affects the reverse-gradient function, which tries to pass a list through unquote. This PR removes this usage of unquote.

Status: Ready to merge

Reviewers: @jeffkamo @avelinet @cole-sanderson @mlworks @yourpalsonja @nastiatikk
Ticket: N/A
Linked PRs: N/A

Changes

  • No longer attempt to unquote the final list returned from reverse-gradient.

How to test-drive this PR

  • Clone this repo and run npm install && bower install to ensure dependencies are installed
  • Run grunt to compile Sass
  • Ensure all the DEBUG statements in output pass
  • Open the test/index.html file and verify that the reverse-gradient function still works.

Research resources

@nastiatikk
Copy link

I didn't know about unquote thing

@nastiatikk
Copy link

So to clarify since Sass 3.4.10 you can pass non-string value only?

@ry5n
Copy link
Contributor Author

ry5n commented May 26, 2015

@nastiatikk Yes, in 3.4.10+ you can’t pass a list to unquote, which we are accidentally doing in the reverse-gradient function. Attempting to unquote a list never did anything, so we can safely remove the call in this case. I’m actually not sure when unquoting is strictly necessary, if ever.

@nastiatikk
Copy link

Maybe @jeffkamo could give some ideas? Seems like it was his tweak 70d0b7b

UPD: But i see it was done by Kyle P originally

@jeffkamo
Copy link
Contributor

I'm fine with this change. If the usecase is no longer supported (via deprecation) it's probably a good idea that we fix our code to reflect that.

@jeffkamo
Copy link
Contributor

I just haven't tested the change yet.

@ry5n
Copy link
Contributor Author

ry5n commented May 26, 2015

@jeffkamo Just to be clear, there was never a “use case” for passing a Sass list to unquote – it just allowed you to do that and passed the list through unchanged. Now, this is deprecated (and will be an error in future versions).

@jeffkamo
Copy link
Contributor

Yup, that's basically what I meant. I suppose I should've said "non-use case" instead.

@ry5n
Copy link
Contributor Author

ry5n commented May 26, 2015

🙇

@nastiatikk
Copy link

This change sounds reasonable
+1

@kpeatt
Copy link
Contributor

kpeatt commented May 27, 2015

I'm sure I must have done that for a reason though... 🐙

@ry5n
Copy link
Contributor Author

ry5n commented May 27, 2015

@kpeatt Hi there 😄. I can see it being done as insurance, in case the list itself could be a string. Re-reading the code again, it seems like for some code paths that could be the case. We do need to change something here, but probably need to be more careful about it, and have a test for it. Let me look at this again and make sure.

@ry5n ry5n closed this May 27, 2015
@ry5n
Copy link
Contributor Author

ry5n commented May 27, 2015

Opened #47 until I can write a more careful PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants