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

Question / feature request: use processor to generate CSS modules #173

Open
marcalexiei opened this issue Nov 21, 2024 · 11 comments · May be fixed by #174
Open

Question / feature request: use processor to generate CSS modules #173

marcalexiei opened this issue Nov 21, 2024 · 11 comments · May be fixed by #174

Comments

@marcalexiei
Copy link
Collaborator

Followup of #166 (comment)

I would like to do another change before the next release:
I'm trying to use processor to generate CSS modules:

    sass({
      processor: async (styles, filePath) => {
        let scopedClassNames = {};
        const postcssProcessResult = await postcss([
          postcssModules({
            getJSON: (_, json) => {
              if (json) scopedClassNames = json;
            },
          }),
        ]).process(styles, {
          from: filePath,
        });

        return { ...scopedClassNames, css: postcssProcessResult.css };
      },

Right now the plugin always returns the css string as export default and I haven't found a clean solution to change it that using processor option.
Have you implemented something similar that I can use or do you know another way?

My possible solution is to allow user to include a new property inside processor output (e.g.: cssModuleMap)
which, if present, will be used as default export.

What do you think?

@elycruz
Copy link
Owner

elycruz commented Dec 2, 2024

Hey @marcalexiei - From what you're proposing it seems you're trying to export the CSS in question as a "named export", is that correct?

@elycruz
Copy link
Owner

elycruz commented Dec 2, 2024

.. essentially, if you could elaborate on the use cases, it would help clarify what we need in this case.

@marcalexiei
Copy link
Collaborator Author

The use case is to allow the plugin to emit CSS modules.
Right now the default export is always the compiled CSS.

@elycruz
Copy link
Owner

elycruz commented Dec 3, 2024

Ok - Question 2. - Doesn't rollup already have rollup css-module plugins? E.g., rollup-plugin-postcss, etc?

@elycruz
Copy link
Owner

elycruz commented Dec 3, 2024

Also, seems something like a cssModules option (on the plugin options object) would be more appropriate, since the processing and outputting of the css is actually slightly different from just output css (will help separate logic, in implementation).

@marcalexiei
Copy link
Collaborator Author

Doesn't rollup already have rollup css-module plugins? E.g., rollup-plugin-postcss, etc?

There are rollup-plugin-postcss and rollup-plugin-postcss-modules They are both un-mainted.


Also, seems something like a cssModules option (on the plugin options object) would be more appropriate,

It would be fine, however I was thinking about using processor output because avoids to add dependencies on the plugin side.


since the processing and outputting of the css is actually slightly different from just output css (will help separate logic, in implementation).

processRenderResponse already handles all the code changes:

if (rollupOptions.insert) {
/**
* Include import using {@link INSERT_STYLE_ID} as source.
* It will be resolved to insert style function using `resolvedID` and `load` hooks;
* e.g., the path will completely replaced, and re-generated (as a relative path)
* by rollup.
*/
imports = `import ${INSERT_STYLE_ID} from '${INSERT_STYLE_ID}';\n`;
defaultExport = `${INSERT_STYLE_ID}(${out});`;
} else if (!rollupOptions.output) {
defaultExport = out;
}
return `${imports}export default ${defaultExport};\n${restExports}`;

My proposed solution is about to add the required logic around this code.

@elycruz
Copy link
Owner

elycruz commented Dec 8, 2024

Ok, I see - Makes sense. In this case please feel free to move ahead with opening a PR.

@Daydreamer-riri
Copy link

This feature is really needed.

@Daydreamer-riri
Copy link

Hi @elycruz, due to the urgent need for this feature, I've implemented it through a patch and have already put it into use. This was achieved by adding a specific field to the return value of the processor (similar to css). If you think this change is appropriate, I can open a PR.

diff --git a/dist/index.js b/dist/index.js
index 8cd7f0a3043f0f6912c03358f500d008b1193504..ba21a04e5190fcdac79fc8f0a64ef7ee0c99e5bd 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -81,9 +81,9 @@ const processRenderResponse = (rollupOptions, file, state, inCss) => {
         const outCss = result.css;
         delete result.css;
         const restExports = Object.keys(result).reduce((agg, name) => agg + `export const ${name} = ${JSON.stringify(result[name])};\n`, '');
-        return [outCss, restExports];
+        return [outCss, restExports, result.cssModules];
     })
-        .then(([resolvedCss, restExports]) => {
+        .then(([resolvedCss, restExports, cssModules]) => {
         const { styleMaps } = state;
         styleMaps[file].content = resolvedCss;
         const out = JSON.stringify(resolvedCss);
@@ -96,6 +96,9 @@ const processRenderResponse = (rollupOptions, file, state, inCss) => {
         else if (!rollupOptions.output) {
             defaultExport = out;
         }
+        if (cssModules) {
+            defaultExport = JSON.stringify(cssModules)
+        }
         return `${imports}export default ${defaultExport};\n${restExports}`;
     }));
 };

@Daydreamer-riri
Copy link

I realize that the current official version prevents me from generating CSS modules because the processor cannot return a default export. So I think perhaps allowing a default export here could solve many problems.

@marcalexiei
Copy link
Collaborator Author

From my point of view your patch might introduce some bugs:

  • it doesn't take into account the scenario where a user specify insert option to true.
    insertStyle utility will never be injected in the code because you are replacing defaultExport which takes care of doing this.
  • In addition, by replacing defaultExport with the css modules, the compiled CSS is no longer present in the generated file which means the user must "manually" extract it via output option?

To wrap up I don't think that this solution is feasible when using rollup preserverModules option and/or insert plugin option.


If you want to open a PR you should consider these scenarios, otherwise you have to wait for a few days...
I was already planning to open a PR to address this issue.

@marcalexiei marcalexiei linked a pull request Dec 10, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants