typescript-plugin-css-modules icon indicating copy to clipboard operation
typescript-plugin-css-modules copied to clipboard

Stop including `sass` and `less` as dependencies

Open alecf opened this issue 5 years ago • 3 comments

Is your feature request related to a problem? Please describe.

Right now both sass and less are direct dependencies of this project, which seem unnecessary. This bloats up the install and if you were already using node-sass, it messes with the css-loader configuration, switching your sass implementation (which happens to not be compatible with blueprint)

There is a workaround to fix css-loader but it seems like an unwanted side effect of installing this package.

Describe the solution you'd like Add them as optionalDependencies

Describe alternatives you've considered None, you can't opt-out, so there's no alternatives

Additional context n/a

alecf avatar Aug 21 '20 20:08 alecf

Hi @alecf, I agree with you - and I assure you that we're doing our best to keep this package lightweight.

Unfortunately, adding them to optionalDependencies would still cause NPM to install them (however, it'll continue to install other dependencies if they fail to install for some reason). So this wouldn't solve the problem.

We could make the other dependencies peerDependencies, but the user would then be required to install them, or this plugin would fail. I'm not sure this is a better DX.

We could also try to get the plugin to work with node-sass, if you're interested in contributing?

mrmckeb avatar Sep 12 '20 14:09 mrmckeb

@mrmckeb

Hi! I recently found out about this package and happily decided to use it. It works fine, but my lock file has grown ~ 1200 lines. This is because I don't use any less or sass at all. As far as I know, many also use vanilla css modules. Therefore, I want to suggest the following scenario, which can be a compromise for everyone.

I propose to split the implementation for different preprocessors, leaving only vanilla css in the main package. Preprocessor users will need to include a package variant for their preprocessor. In this case, preprocessor dependencies can be moved to the peer. Usually, the need to install such dependencies is written in the documentation. In this case, it will not make sense, because the user will most likely already have the preprocessor.

How it will look for users of vanilla css: everything will remain the same, only there will be fewer dependencies.

How it will look to users of the preprocessor (e.g. less): " plugins ": [{" name ":" typescript-plugin-css-modules "}] should be replaced by " plugins ": [{" name ":" typescript-plugin-css-modules / less "} ] ; less should be installed manually.

Of course, this will be a breaking change, which will also require writing a little migration guide and updating the major version.

Also it won't help in case of duplicate sass and node-sass (but node-sass is deprecated, so I think this problem is not so important).

dartess avatar Jul 14 '21 14:07 dartess

I can think of a couple of ways to do this. Both are major breaking changes, but might get us a solution to this:

Option A: Split up the package into a separate package for each processor

You would effectively end up with:

  • typescript-plugin-css-modules
  • typescript-plugin-css-modules-stylus
  • typescript-plugin-css-modules-less
  • typescript-plugin-css-modules-sass
  • typescript-plugin-css-modules-stylus

That would allow the dependencies of each package to only bring in the preprocessor that is needed. There might be a challenge of figuring how out how to ensure multiple packages can be loaded for a project as I imagine there are crazy people out there who might have a mixture of LESS, SASS and Stylus all being used at once.

Option B: Bring your own preprocessor

Refactor typescript-plugin-css-modules to allow users to pass in the path to a preprocessor they already have installed as an option. I'm imagining a new option, something like this:

{
  "compilerOptions": {
    "plugins": [
      {
        "name": "typescript-plugin-css-modules",
        "options": {
            "preprocessor": "./node_modules/stylus"
        }
      }
    ]
  }
}

This would then instruct typescript-plugin-css-modules to run the file through that preprocessor before validating the types. It has the added benefit of re-using whatever dependency the user already has installed which means it would use whatever version of that preprocessor the user is using (at the moment typescript-plugin-css-modules installs its own version of Stylus/Less/Sass whereas the user might be using a different version).

I'm not sure if this option is technically viable as the syntax for running each preprocessor is slightly different, so it might not be as easy to make the code reliably work for all versions of all preprocessors. If this proves to be too challenging, we could instead be more explicit with the option (ie. lessPreprocessor/sassPreprocessor vs. preprocessor).

@mrmckeb Do you think either of these options would be viable?

EvHaus avatar Jul 08 '22 03:07 EvHaus

Instead of using sass and less directly, it could use the postcss-sass and postcss-less plugins to parse the files. We don't really need it to convert the sass/less file to css, only to be able to parse it and generate some ast, and using these seem to work:

import postcss from "postcss";
import postcssIcssSelectors from "postcss-icss-selectors";
import postcssLess from "postcss-less";

const processor = postcss([
  postcssIcssSelectors({
    mode: 'local'
  })
]);

const content = await readFile('style.less', 'utf-8');

const result = processor.process(content, {
  syntax: postcssLess,
  from: 'style.css',
  map: {
    inline: false,
    prev: false
  }
});

mariusGundersen avatar Jan 17 '23 13:01 mariusGundersen

That's a good idea @mariusGundersen. My only concern around that is that a plugin like that is reimplementing Sass/Less, and may fall behind or have bugs.

mrmckeb avatar Feb 18 '23 06:02 mrmckeb

I tried it a bit more, and unfortunately it won't work. The tests have advanced features of less and sass that require the full compiler to work (generating classes in loops for example). I don't use those advanced features in my code, so it's not a problem for me, but it will be for others

mariusGundersen avatar Feb 18 '23 10:02 mariusGundersen

I'll close this off for now, as I'm not sure what we can do other than to split this into smaller packages (which is something I'm considering).

mrmckeb avatar Sep 22 '23 05:09 mrmckeb