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

Remove math.div warnings when compiling components #385

Merged
merged 3 commits into from
Nov 3, 2021
Merged

Conversation

JakeChampion
Copy link
Contributor

@JakeChampion JakeChampion commented Oct 29, 2021

This PR introduces a temporary helper library named @financial-times/math to avoid newer dart-sass versions printing warnings when compiling Origami.

The library contains a div function which is defined twice, once in index.scss and again in index.import.scss

When compiling using dart-sass, the index.import.scss file will be used, which implements the div function with the new math.div if supported and with / if not supported.

When compiling using lib-sass or ruby-sass, the index.scss file will be used, which implements the div function using the / operator (which is deprecated in dart-sass)

As well as a div function, the library also re-exports all the functions from mathsass but re-implements the ones which used the / operator, such as pow and sqrt.

I've also added an internal fork of sass-mq which makes use of @financial-times/math to remove the last remaining sass warnings which happen

@JakeChampion JakeChampion temporarily deployed to origami-pr-385 November 1, 2021 18:59 Inactive
@JakeChampion JakeChampion temporarily deployed to origami-pr-385 November 1, 2021 19:07 Inactive
@github-actions github-actions bot removed the templates label Nov 1, 2021
@JakeChampion JakeChampion temporarily deployed to origami-pr-385 November 2, 2021 13:46 Inactive
@JakeChampion JakeChampion temporarily deployed to origami-pr-385 November 3, 2021 11:09 Inactive
This commit introduces a temporary helper library to avoid newer dart-sass versions printing warnings when compiling Origami.

The library contains a `div` function which is defined twice, once in `index.scss` and again in `index.import.scss`

When compiling using dart-sass, the `index.import.scss` file will be used, which implements the `div` function with the new `math.div` if supported and with `/` if not supported.

When compiling using libsass or rubysass or node-sass, the `index.scss` file will be used, which implements the `div` function using the `/` operator (which is deprecated in dartsass)

As well as a `div` function, the library also re-exports all the functions from `mathsass` but re-implements the ones which used the `/` operator, such as `pow` and `sqrt`.
@JakeChampion
Copy link
Contributor Author

@Financial-Times/origami-core This is now ready for a review :-D

<img width="500" height="500" src="https://avatars3.githubusercontent.com/u/9341289?v=3&s=500" alt="Awesome">
</div>

# Media Queries with superpowers [![Build Status](https://api.travis-ci.org/sass-mq/sass-mq.svg?branch=master)](https://travis-ci.org/sass-mq/sass-mq)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be great to add a note here as to why we're forking it, as in the math library :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 71a92c0 (#385)

Comment on lines +32 to +34
// math.div was added in version 1.33.0
// import-only files were added in version 1.23.0
// meta.module-functions was added in version 1.23.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

Copy link
Contributor

@notlee notlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very clever way to remove these very frustrating warnings :D

@JakeChampion JakeChampion temporarily deployed to origami-pr-385 November 3, 2021 15:31 Inactive
@JakeChampion JakeChampion enabled auto-merge (rebase) November 3, 2021 15:35
@JakeChampion JakeChampion merged commit d09fffa into main Nov 3, 2021
@JakeChampion JakeChampion deleted the div branch November 3, 2021 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants