sass-extract-loader icon indicating copy to clipboard operation
sass-extract-loader copied to clipboard

Allow passing compile options (other than file) to `render`

Open jrencz opened this issue 8 years ago • 10 comments

I'd like for sass-extract-loader to be able to consume the same paths sass-loader can. I have sass-loader configured in a way that it has custom includePaths defined and it has some custom functions defined.

I'd like to be able to pass the same sass config for both plugins since even if I reach the desired import using relative path (instead of relying on include paths) I then bump into a problem of undefined functions

jrencz avatar Jul 14 '17 07:07 jrencz

I tried enabling the config the simplest way:

diff --git a/index.js b/index.js
index 9942d34..2aaf8df 100644
--- a/index.js
+++ b/index.js
@@ -4,7 +4,11 @@ module.exports = exports = function sassExtractLoader(content) {
   const callback = this.async();
   this.cacheable();
 
-  return sassExtract.render({ file: this.resourcePath })
+  const options = this.options.sassExtractLoader;
+
+  return sassExtract.render(Object.assign({}, options, {
+    file: this.resourcePath,
+  }))
   .then(rendered => {
     this.value = [rendered.vars];
     const result = `module.exports = ${JSON.stringify(rendered.vars)};`;

My config is:

const path = require('path');

const babelrc = require('rc')('babel');
const getPluginConfig = (plugins, name) => {
  const pluginDeclarations = plugins.filter(plugin => Array.isArray(plugin) ?
    plugin[0] === name :
    plugin === name
  );

  if (pluginDeclarations.length > 1) {
    throw new Error(`Misconfiguration? More than one plugin named ${ name }`);
  }

  return Array.isArray(pluginDeclarations[0]) ?
    pluginDeclarations[0][1] :
    null;
};

module.exports = {
  includePaths: [
    ...getPluginConfig(babelrc.plugins, 'module-resolver')
      .root
      .map(pathName => path.resolve(pathName)),
    path.resolve('./node_modules/'),
  ],
};

I use babel to resolve modules. Here I share config between babel and sass.

I get error:

Module build failed: Error: Cannot read property 'injectedData' of undefined
    at options.error (/Volumes/Projects/opensource/sass-extract-loader/node_modules/node-sass/lib/index.js:291:26)

Sadly: this is the 1st an only step above libsass. I can't find the problem.

jrencz avatar Jul 14 '17 12:07 jrencz

@jrencz Yes this is due to a problem with the importer, there will be a new version of the loader and sass-extract out today which will fix these issues.

jgranstrom avatar Jul 14 '17 13:07 jgranstrom

@jrencz there's now a new version of [email protected] and [email protected] that will allow you to use includePaths, or any other options that you may want to pass to the compilation. There is an example available in the repo.

npm install [email protected] && npm install [email protected]

jgranstrom avatar Jul 14 '17 15:07 jgranstrom

@jgranstrom It looks like webpack won't allow passing function references through query. It's impossible to pass importer option this way. Webpack strips all keys that contain functions. I guess it's because Webpack query is JSON.stringified and it's JSON.stringify who removes functions.

(I'm testing on Webpack 1 btw. I don't know how 2 and 3 are behaving)

I suppose it would be OK to use the same approach sass-loader uses, namely: allowing to define key names after the loader which won't be restricted to the same rules query is.

jrencz avatar Jul 17 '17 10:07 jrencz

@jrencz yes this is definitely a bit of an issue with the way webpack passes the config. I will look into adding an option that allows passing an importer function.

jgranstrom avatar Jul 17 '17 12:07 jgranstrom

Another thing is that sass-extract has its own importer. If importer is given both should work

jrencz avatar Jul 17 '17 12:07 jrencz

Yes, importers can actually be provided as an array to be executed in order. I will add some testing for when providing an importer that likely is expected to run before the extract importer. What is it that your custom importer is doing different to the default one?

jgranstrom avatar Jul 17 '17 12:07 jgranstrom

glob pattern resolution

jrencz avatar Jul 17 '17 12:07 jrencz

For the sake of clarification: I do use an importer for glob pattern resolution (this one: node-sass-globbing) but it's not used in files I currently want to process with this loader.

jrencz avatar Jul 17 '17 17:07 jrencz

@jrencz got it, thank you for clarifying. I will use it as an example use case for testing

jgranstrom avatar Jul 18 '17 08:07 jgranstrom