Repository: tivac/modular-css
Pull request #901 (2022-10-14 12:42:48)

[css-to-js] option to change type of variable declaration
<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->
New option for `css-to-js` to change a kind of variable declaration, defaults to `const`.

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Some folks still have to support ES5, thus `const` declarations within JS output may not help.
Naturally, we can process modules by an additional loader (e.g. babel-loader) but it involves significant overhead.
To avoid extra loaders I'd like to introduce a new option.

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
Unit tests have been updated.

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the **CONTRIBUTING** document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: f2541693a1963a65873057e4db623429953246fc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/plesiecki/modular-css/new/css-to-js-variableDeclaration-option?filename=.changeset/few-ravens-protect.md&value=---%0A%22%40modular-css%2Fcss-to-js%22%3A%20patch%0A%22%40modular-css%2Fwebpack%22%3A%20patch%0A---%0A%0AWEBPERF-2906%20%5Bcss-to-js%5D%20option%20to%20change%20type%20of%20variable%20declaration%0A)


netlify[bot]: ### <span aria-hidden="true">✅</span> Deploy Preview for *m-css* ready!


|  Name | Link |
|---------------------------------|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | f2541693a1963a65873057e4db623429953246fc |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/m-css/deploys/634959491f6689000848d535 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-901--m-css.netlify.app](https://deploy-preview-901--m-css.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTkwMS0tbS1jc3MubmV0bGlmeS5hcHAifQ.eiLlOi5QmAGppwKmjHIHoTmpCUB-JWwEkCOGzctY5iE)<br /><br />_Use your smartphone camera to open QR code link._</details> |
---

_To edit notification comments on pull requests, go to your [Netlify site settings](https://app.netlify.com/sites/m-css/settings/deploys#deploy-notifications)._
tivac: @plesiecki One question but otherwise looks good. I didn't want to have to add yet another option, but the reasoning makes sense to me. Does webpack not handle this sort of transform automatically if your output target doesn't support `const`?

One request though: Can you add a changeset please?
plesiecki: > Does webpack not handle this sort of transform automatically if your output target doesn't support `const`?
Setting target like `target: ['web', 'es5']` affects webpack's runtime only, it doesn't do anything with user's code unfortunately.

codecov[bot]: # [Codecov](https://codecov.io/gh/tivac/modular-css/pull/901?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) Report
Base: **99.71**% // Head: **99.71**% // Increases project coverage by **`+0.00%`** :tada:
> Coverage data is based on head [(`75d6d20`)](https://codecov.io/gh/tivac/modular-css/pull/901?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) compared to base [(`e026d81`)](https://codecov.io/gh/tivac/modular-css/commit/e026d812b17526a8f7e756205f5ed7424563dfee?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main     #901   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files          37       37           
  Lines        1425     1426    +1     
  Branches      232      232           
=======================================
+ Hits         1421     1422    +1     
  Misses          4        4           
```


| [Impacted Files](https://codecov.io/gh/tivac/modular-css/pull/901?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit) | Coverage Δ | |
|---|---|---|
| [packages/css-to-js/css-to-js.js](https://codecov.io/gh/tivac/modular-css/pull/901/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit#diff-cGFja2FnZXMvY3NzLXRvLWpzL2Nzcy10by1qcy5qcw==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/tivac/modular-css/pull/901?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pat+Cavit).


Patch:
diff --git a/.changeset/chatty-turtles-look.md b/.changeset/chatty-turtles-look.md
new file mode 100644
index 00000000..6dc71494
--- /dev/null
+++ b/.changeset/chatty-turtles-look.md
@@ -0,0 +1,8 @@
+---
+"@modular-css/css-to-js": minor
+"@modular-css/rollup": minor
+"@modular-css/vite": minor
+"@modular-css/webpack": minor
+---
+
+New option to change type of variable declaration — `variableDeclaration` which defaults to `const`. Can be set to `var` if necessary, e.g. ES5 support.
diff --git a/packages/css-to-js/css-to-js.js b/packages/css-to-js/css-to-js.js
index 0dfba4da..32d03135 100644
--- a/packages/css-to-js/css-to-js.js
+++ b/packages/css-to-js/css-to-js.js
@@ -14,11 +14,11 @@ const slash = (file) => file.replace(/\/|\\/g, "/");
 const DEFAULT_VALUES = "$values";
 
 const DEFAULTS = {
-    __proto__   : null,
-    dev         : false,
-    styleExport : false,
-
-    namedExports : {
+    __proto__           : null,
+    dev                 : false,
+    styleExport         : false,
+    variableDeclaration : "const",
+    namedExports        : {
         rewriteInvalid : true,
         warn           : true,
     },
@@ -66,6 +66,8 @@ exports.transform = (file, processor, opts = {}) => {
         warnOnInvalid = options.namedExports;
     }
 
+    const { variableDeclaration } = options;
+
     const { graph } = processor;
 
     const id = processor.normalize(file);
@@ -209,7 +211,7 @@ exports.transform = (file, processor, opts = {}) => {
 
         internalsMap.set(value, unique);
 
-        out.push(`const ${unique} = ${JSON.stringify(value)}`);
+        out.push(`${variableDeclaration} ${unique} = ${JSON.stringify(value)}`);
 
         valueExports.set(key, unique);
     });
@@ -217,7 +219,7 @@ exports.transform = (file, processor, opts = {}) => {
     if(valueExports.size) {
         const unique = deconflict(identifiers, DEFAULT_VALUES);
 
-        out.push(dedent(`const ${unique} = {
+        out.push(dedent(`${variableDeclaration} ${unique} = {
             ${[ ...valueExports ].map(prop).join(",\n")},
         };`));
 
@@ -255,7 +257,7 @@ exports.transform = (file, processor, opts = {}) => {
 
         elements.push(...details.classes[key].map((t) => JSON.stringify(t)));
 
-        out.push(`const ${unique} = ${elements.join(` + " " + `)}`);
+        out.push(`${variableDeclaration} ${unique} = ${elements.join(` + " " + `)}`);
 
         defaultExports.push([ key, unique ]);
 
diff --git a/packages/webpack/README.md b/packages/webpack/README.md
index 8d19b4e7..ac62f110 100644
--- a/packages/webpack/README.md
+++ b/packages/webpack/README.md
@@ -72,7 +72,7 @@ All other options are passed to the underlying `Processor` instance, see [Option
 
 #### `styleExport`
 
-You can enable returning the style string, eg `import { styles } from "./style.css";`. by setting `styleExport` to `true`.
+You can enable returning the style string, eg `import { styles } from "./style.css";`, by setting `styleExport` to `true`.
 
 ```js
 ...
@@ -82,7 +82,27 @@ You can enable returning the style string, eg `import { styles } from "./style.c
             use  : {
                 loader  : "@modular-css/webpack/loader",
                 options : {
-                    styleExport : true
+                    styleExport : true,
+                }
+            }
+        }]
+    },
+...
+```
+
+#### `variableDeclaration`
+
+You can set variable declaration kind, eg `var mc_rule = ...;`, by setting `variableDeclaration` to `var`. Defaults to `const`.
+
+```js
+...
+    module : {
+        rules : [{
+            test : /\.css$/,
+            use  : {
+                loader  : "@modular-css/webpack/loader",
+                options : {
+                    variableDeclaration : "var",
                 }
             }
         }]


Test patch:
diff --git a/packages/css-to-js/test/__snapshots__/api.test.js.snap b/packages/css-to-js/test/__snapshots__/api.test.js.snap
index 8626e746..b9915f4e 100644
--- a/packages/css-to-js/test/__snapshots__/api.test.js.snap
+++ b/packages/css-to-js/test/__snapshots__/api.test.js.snap
@@ -71,6 +71,23 @@ custom
 };
 `;
 
+exports[`@modular-css/css-to-js API should generate javascript with var variable statements: code 1`] = `
+var str = "\\"string\\""
+var $values = {
+    str,
+};
+var fooga = "mce1ef44f3_fooga"
+export default {
+    $values,
+fooga
+};
+
+export {
+    $values,
+fooga
+};
+`;
+
 exports[`@modular-css/css-to-js API should generate javascript: code 1`] = `
 const a = "mc74f3fa7b_a"
 export default {
diff --git a/packages/css-to-js/test/api.test.js b/packages/css-to-js/test/api.test.js
index cce82971..8eb760ec 100644
--- a/packages/css-to-js/test/api.test.js
+++ b/packages/css-to-js/test/api.test.js
@@ -186,6 +186,16 @@ describe("@modular-css/css-to-js API", () => {
         expect(code).toMatchSnapshot("code");
     });
 
+    it("should generate javascript with var variable statements", async () => {
+        const processor = new Processor({ resolvers });
+
+        await processor.file(require.resolve("./specimens/simple.css"));
+
+        const { code } = transform(require.resolve("./specimens/simple.css"), processor, { variableDeclaration : "var" });
+
+        expect(code).toMatchSnapshot("code");
+    });
+
     it.each([
         true,
         false,