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

fix(styles): revisit sass restriction remediation #13938

Closed
wants to merge 7 commits into from

Conversation

mcdmaster
Copy link

Description

Revisit remediation of PR #13704
Fixes #13694
Fixes Tokyo-Metro-Gov/covid19#6346

Motivation and Context

Since it is figured out that the problem was caused by insufficiency fo fixing errors, SASS compilation error took place on the previous SASS-mathdiv facility in the PR shown above.
It is inevitable for us to fix this soon to let rhe new SASS restrictions / lintings come in .

How Has This Been Tested?

Check, build & run to see at least no similar error to previus is produced when monitoring the behavior, and the application wchich uses Vuetify can work as welll as before the program fix.
Also checked the build log that no SASS-related errors were produced:
スクリーンショット 2021-07-18 222335

Markup:

<details>
<template>
  <v-container>
    <!--  -->
  </v-container>
</template>

<script>
  export default {
    data: () => ({
    //
    }),
  }
</script>
</details>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@KaelWD
Copy link
Member

KaelWD commented Jul 18, 2021

@mcdmaster
Copy link
Author

mcdmaster commented Jul 18, 2021

I knew. Hence, this commit is aiming to overcome the unsuccessful & cyclical approach that had been taken. I hope this helps you and the team to decide go/nogo with this commit:

The former, we needed to "centralize" the declaration of '@use "sass:math"'. Nevertheless, release 2.5.2 has redundant declarations, which had been done due to try to fulfill coverage in an inappropriate way.

If you mean, in the latter, that you were struggling with unwanted stoppages reported, this commit may give its countermeasure wholly or partially, including "centralization".

In practical, the '@use' directive can only be declared once i.e. in the topmost '_variables.scss' file (or another). Then the 'div()' method is enabled its use in low-level stylesheet implementations as if it were global.

These are what I have tried since previous. Hope this helps.

@KaelWD
Copy link
Member

KaelWD commented Jul 29, 2021

the '@use' directive can only be declared once i.e. in the topmost '_variables.scss' file (or another). Then the 'div()' method is enabled its use in low-level stylesheet implementations as if it were global

This clearly is not the case, if you look at the first test deployment there are raw div() expressions being emitted in the compiled CSS.

This PR doesn't even build anymore after b6ca8e9, if you do have another solution to this please test it locally first by running yarn build && yarn dev docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid The issue is missing information or is not a valid bug/feature request sass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SASS / SCSS の割り算の記法 [Bug Report][2.5.0] New Sass Deprecation Recommended
3 participants