ionic-team / stencil-sass

Sass plugin for Stencil
https://www.npmjs.com/package/@stencil/sass
Other
75 stars 23 forks source link

using custom sass functions not working #24

Open quackes opened 4 years ago

quackes commented 4 years ago

Sass offers the api to register custom functions to use in sass stylesheets (https://sass-lang.com/documentation/js-api#functions)

These do not work due to the fact that stencil-sass comes with a packed version of dart-sass

Steps to reproduce

stencil.config.ts

import { Config } from '@stencil/core';
import {sass} from "@stencil/sass";
import {types} from "sass";

export const config: Config = {
  namespace: 'my-components',
  globalStyle: 'src/global/style.scss',
  plugins: [
    sass({
      functions: {
        'my-function($param)': function(param, done) {

           // do something fancy with param....
          return new types.String(param));
        }
      }
    })
  ]
};

src/global/style.scss

:root {
  --myvar:  my-function('my-param')
}

when I run the build the following error occurs

[ ERROR ]  sass error:src/global/style.scss:
           'my-param' must be a Sass value type.

Cause

The error occurs because dart-sass checks the type with instanceof, and the called constructor in the custom function implementation is not the same as the constructor checked in the bundled dart-sass code in stencil-sass

I "solve" the error reexporting the dart-sass object in stencil-sass and using the types from that module. Thats not a viable workaround, more a proof of the error cause.

Suggestions

1

I don't know, why dart-sass is bundled with stencil-sass, but one solution would be to skip the bundling and load it as dependency instead of devDependency

2

Reexport the dart-sass instance in order enable the use dart-sass api outside of stencil-sass However, this would limit the ability to use 3rd-party modules that use dart-sass

nekator commented 4 years ago

you can use this sass plugin as alternative. It uses node-sass instead of sass(dart-sass) and requires node-sass as peerDependency.

I know this is no real solution to your problem but you can use this sass-plugin util the real problem is fixed.

anthonylebrun commented 4 years ago

@nekator thanks, that worked but I do miss the latest the latest sass features :(

jcamargodev commented 3 years ago

I have this problem too, did anyone find a solution for this?
I can't lose the functionality of dart-sass so using the suggested tip is unfortunately not a solution :(

@quackes Do you have any news on this subject?

tfrijsewijk commented 3 years ago

I got around this limitation as well, even for writing my own functions this is a problem. I had to re-export the instance as well to get access to the correct types.

I persisted the changes using https://www.npmjs.com/package/patch-package, so I can use this in CI/CD as well. It sure isn't pretty and I recommend caution for those doing this as well.

So, why is dart-sass bundled anyway?

KevinCarnaille2 commented 2 years ago

Since 2019 there are no decent way to fix it ? I'm not sure to understand what you mean by "reexport the instance" though ?

quackes commented 2 years ago

I mean, that the instance of dart-sass can be imported by stencil. that way we could use it in custom functions. not the best solution, because you could not use functions from other modules.

imho the best solution is not to use bundled dart-sass.

sprockow commented 2 years ago

Had the same problem when updating sass function to from node-sass to dart-sass. The function pulled it's own version of sass from its node_modules, and the instance of checks failed. I solved this by wrapping the imported function in another function, and ensured that the return value used the same sass module my build tool was using

            "svg-icon($path, $selectors: null)": (() => {
              const inlinerFn = inliner(
                "./"
                [
                  {
                    optimize: true,
                    encodingFormat: "uri",
                  },
                ]
              );

              return (...args) => {
                const output = inlinerFn(...args);
                // serialize sass.type.String to a basic string, then rewrap in another version of sass.type.String
                return new sass.types.String(output.toString());
              };
            })(),
tfrijsewijk commented 2 years ago

I'm not sure to understand what you mean by "reexport the instance" though ?

@KevinCarnaille2: I'm sorry, I completely missed your question!

I modified the @stencil/sass package inside the node_modules folder. Using patch-package (an NPM package) I'm able to persist/reapply these changes whenever package.json is installed (in my case using yarn install).

Arlien commented 2 weeks ago

+1

This is still occurring, is there any hope of it ever being solved ?