[BUGFIX] Fix @extend an outer selector from within @media

[marvinhuebner]

Fixes #87

This fixes two erros with Error: You may not @extend an outer selector from within @media..(https://github.com/SnowdogApps/magento2-theme-blank-sass/issues/87#issuecomment-372298221).

[Igloczek]

Remember to follow the contribution guidelines and fix this bug also in the magento2 repository or provide a proof that was just a mistake translating LESS to SASS. We don't want to create an alternative version of the Blank theme, it needs to be to same no matter if you are usong LESS or SASS

[marvinhuebner]

In the Magento blank theme this is working because LESS is less strict as SASS.

I fully agree that this blank theme needs to do the same as the Magento LESS Theme does. But how to handle the internal differences between LESS and Sass?

To verify that the error stays not in connection with frontools i also tested to compile the theme with node-sass@4.8.2 (node-sass styles/styles.scss -o styles) and the command line compiler. But if i do it this way, i also get the same erros:

{
  "status": 1,
  "file": "/Users/marvinhuebner/Projekte/wot/multistore/vendor/snowdog/theme-blank-sass/styles/blocks/_extends.scss",
  "line": 1133,
  "column": 5,
  "message": "You may not @extend an outer selector from within @media.\nYou may only @extend selectors within the same directive.\nFrom \"@extend .abs-checkout-tooltip-content-position-top-mobile\" on line 137 of Magento_Checkout/styles/module/checkout/_tooltip.scss\n",
  "formatted": "Error: You may not @extend an outer selector from within @media.\n       You may only @extend selectors within the same directive.\n       From \"@extend .abs-checkout-tooltip-content-position-top-mobile\" on line 137 of Magento_Checkout/styles/module/checkout/_tooltip.scss\n        on line 1133 of styles/blocks/_extends.scss\n>>     .abs-checkout-tooltip-content-position-top-mobile {\n\n   ----^\n"
}

So if i understand you correct we have to patch the Magento LESS blank theme so, that it can work the same way in the SASS theme?

I would love to get this fixed in some way, because if i install the blank theme in the current version i can not compile the SASS before i patch this two files.

[Igloczek]

I want to prevent a situation where translating LESS code from M2 repository will bring bugs to the SASS implementations, just b/c LESS is okay with stupid solutions.

So you need to submit PR to the M repository to reflect changes done here.

https://github.com/magento/magento2/blob/2.2-develop/app/design/frontend/Magento/blank/Magento_Checkout/web/css/source/module/checkout/_tooltip.less#L144 https://github.com/magento/magento2/blob/2.2-develop/app/design/frontend/Magento/blank/Magento_Sales/web/css/source/_module.less#L265

[marvinhuebner]

I will close this for now, because i've installed a new vendor and now it works out of the box. I can not explain it to me. Will keep an eye on it.

But thanks for your input and the insights to the SASS port.

[Igloczek]

@talalus let's take this over, I believe we need this fix

[rglepper]

Hi guys, I just wanted to point out that this error is expected since in Sass it's not allowed to use @extend within directives unless the extended selector is within the same block http://sass-lang.com/documentation/file.SASS_REFERENCE.html#extend_in_directives I don't quite understand why this was compiling without errors, but this is definitely a bug.

[marvinhuebner]

I don't quite understand why this was compiling without errors, but this is definitely a bug.

@rglepper i've also no clue why this works. Yesterday we configured our CI/CD for our new project and the build job failed everytime when compiling the SASS with the same error mentioned above. Then we switched from npm to yarn install to install the dependencies depending on the yarn.lock file and then it works.

[rglepper]

@marvinhuebner I think this could be related to the version of libsass being used by node-sass http://sass-compatibility.github.io/#cross_media_extend I know that it doesn't work with the current version of node-sass 4.8.3 and this is the correct behaviour

[marvinhuebner]

@rglepper can confirm. If i update to node-sass 4.8 it breaks. In node-sass 4.8, libsass is upgraded to the latest 3.5 release.

So how can we deal with this? I can not image how i can "fix" this in the less version of the base theme, because if i argue with: "Sass is stricter then less and in sass this does not work anymore, we have to change this in the base theme so the sass theme can adopt this change and the sass compiler will work again".

Personally I think that a patch for the less base theme is hard to become in.

[Igloczek]

Fixed in 1.0.5

Changes are reflected in https://github.com/magento/magento2/pull/14395

[marvinhuebner]

@Igloczek Thanks!