Open adi518 opened 4 years ago
Hi @adi518!
Sorry to hear you're running into an issue. To help us best begin debugging the underlying cause, it is incredibly helpful if you're able to create a minimal reproduction. This is a simplified example of the issue that makes it clear and obvious what the issue is and how we can begin to debug it.
If you're up for it, we'd very much appreciate if you could provide a minimal reproduction and we'll be able to take another look.
Sure, I have a repro in hand. Will post it here soon. 👍
I'm guessing you're referring to tree-shaking in webpack specifically?
Tree-shaking in every tree-shaking compatible tool (Webpack, Rollup, etc'). The issue here is that styleInject
snippets (function calls) are bundled into the global scope without the pure annotation, which means they are detected as side-effects, thus breaking the shake. So, I added the annotation myself by passing a custom inject function. Alas, the entire snippet is stripped out completely, for unknown reasons. Someone who's more savvy with Rollup will probably know, hence the issue. I'll provide the repro soon.
Ah, I didn't know Rollup supported these annotations, but it looks like it does: https://rollupjs.org/guide/en/#treeshake
I guess the problem here is that injecting CSS IS a side effect. I guess injectStyle should only be stripped out if it's injecting styles that are relevant to the JS module requiring them (like a React component). For example, a CSS Module with no :global selector can be safely marked as pure.
Is Webpack's css-loader handling this in some way? How do they do it?
Injecting is a side-effect yes, but we can tell the transpiler it's safe by adding the pure annotation. Babel does that automatically for React imports. Webpack doesn't add shaking measures when you build a library with it, it only knows how to read them, that's why we need Rollup. Webpack 5 might bring these features, but it's not there yet, so until then, we depend on Rollup or other ESM-aware bundlers.
Any updates?
Hi, no. I was busy with other stuff. I intend to come back to this though, because I really need to solve this issue.
Just pinging this thread to see if there is any progress. This would be so great!
Currently, the output from this plugin looks like this:
var css_248z = ".___2X_Ir{background-color:#4e707a;font-size:24px;color:#fff;cursor:pointer}";
var Button_module = {"button":"___2X_Ir"};
styleInject(css_248z);
If we want the styleInject(..)
function to only execute when the export is imported, we need to make the following modifications:
styleInject(..)
function call in an IIFE/*#__PURE__*/
annotationvar Button_module = {"button":"___2X_Ir"};
, we need to return Button_module
from the IIFEButton_module
and change the name of the style mapping variable to styleMapping
.The code would look like this:
var css_248z = ".___2X_Ir{background-color:#4e707a;font-size:24px;color:#fff;cursor:pointer}";
var styleMapping = {"button":"___2X_Ir"};
var Button_module = /*#__PURE__*/(function() {
styleInject(css_248z);
return styleMapping;
})();
These changes can be made inside of the src/postcss-loader.js
file, and it can be put behind a flag (e.g. noSideEffects
). In order to produce the IIFE code above, this is the code I used inside of src/postcss-loader.js
:
if (shouldExtract) {
output += `export default ${JSON.stringify(modulesExported[_this.id])};`;
extracted = {
id: _this.id,
code: result.css,
map: outputMap
};
} else {
const module = supportModules ? JSON.stringify(modulesExported[_this.id]) : cssVariableName;
output += `var ${cssVariableName} = ${JSON.stringify(result.css)};\n` + `var styleMapping = ${module};\n` + `export var stylesheet=${JSON.stringify(result.css)};`;
}
if (!shouldExtract && shouldInject) {
output += typeof options.inject === 'function' ? options.inject(cssVariableName, _this.id) : '\n' + `import styleInject from '${styleInjectPath}';\n` + `export default /*#__PURE__*/(function() {\n styleInject(${cssVariableName}${Object.keys(options.inject).length > 0 ? `,${JSON.stringify(options.inject)}` : ''});\n return styleMapping;\n})();`;
}
For others looking at this comment, I also added a feature request to a similar plugin: https://github.com/Anidetrix/rollup-plugin-styles/issues/168
Currently, the output from this plugin looks like this:
var css_248z = ".___2X_Ir{background-color:#4e707a;font-size:24px;color:#fff;cursor:pointer}"; var Button_module = {"button":"___2X_Ir"}; styleInject(css_248z);
If we want the
styleInject(..)
function to only execute when the export is imported, we need to make the following modifications:
- Wrap the
styleInject(..)
function call in an IIFE- Use the
/*#__PURE__*/
annotation- Instead of exporting
var Button_module = {"button":"___2X_Ir"};
, we need to returnButton_module
from the IIFE- Set the result of the IIFE to a variable. We can call this variable
Button_module
and change the name of the style mapping variable tostyleMapping
.The code would look like this:
var css_248z = ".___2X_Ir{background-color:#4e707a;font-size:24px;color:#fff;cursor:pointer}"; var styleMapping = {"button":"___2X_Ir"}; var Button_module = /*#__PURE__*/(function() { styleInject(css_248z); return styleMapping; })();
These changes can be made inside of the
src/postcss-loader.js
file, and it can be put behind a flag (e.g.noSideEffects
). In order to produce the IIFE code above, this is the code I used inside ofsrc/postcss-loader.js
:if (shouldExtract) { output += `export default ${JSON.stringify(modulesExported[_this.id])};`; extracted = { id: _this.id, code: result.css, map: outputMap }; } else { const module = supportModules ? JSON.stringify(modulesExported[_this.id]) : cssVariableName; output += `var ${cssVariableName} = ${JSON.stringify(result.css)};\n` + `var styleMapping = ${module};\n` + `export var stylesheet=${JSON.stringify(result.css)};`; } if (!shouldExtract && shouldInject) { output += typeof options.inject === 'function' ? options.inject(cssVariableName, _this.id) : '\n' + `import styleInject from '${styleInjectPath}';\n` + `export default /*#__PURE__*/(function() {\n styleInject(${cssVariableName}${Object.keys(options.inject).length > 0 ? `,${JSON.stringify(options.inject)}` : ''});\n return styleMapping;\n})();`; }
For others looking at this comment, I also added a feature request to a similar plugin: Anidetrix/rollup-plugin-styles#168
Thank you so much for that! I created a pull request, let's hope they merge it.
This function doesn't have a
/*#__PURE__*/
prefix, which breaks tree-shaking. Moreover, when I try to prepend it myself throughinject
option it gets stripped along with the entire string from the final bundle, which leaves you without CSS.In my case, I forked
styleInject
, because it's missing some features, so I had to compose it slightly different into the string (wrap it with brackets to turn it into expression, which is then invokable).