Repository: tivac/modular-css
Pull request #574 (2019-03-28 06:31:49)

feat: replacing missing css.fooga with stringified version
<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->
When a `{css.fooga}` reference (or just `css.fooga` in `<script>` blocks) is found, it's now replaced with `"css.fooga"`.

## 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. -->
So that svelte generates less code and the code it does generate doesn't explode. There's still `strict` mode if you want explosions though!

This helps #571 but I'm not sure it fixes it completely.

## 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. -->
Added tests!

## 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)
- [ ] New feature (non-breaking change which adds functionality)
- [x] 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! -->
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the **CONTRIBUTING** document.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.


TravisBuddy: Hey @tivac,
Something went wrong with the build.

TravisCI finished with status `errored`, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

<a href="https:&#x2F;&#x2F;travis-ci.org&#x2F;tivac&#x2F;modular-css&#x2F;builds&#x2F;512384690">View build log</a>

###### TravisBuddy Request Identifier: f9517d70-5158-11e9-ad12-835a6074a428


Patch:
diff --git a/packages/processor/plugins/values-composed.js b/packages/processor/plugins/values-composed.js
index a39c6ef2..459ceca3 100644
--- a/packages/processor/plugins/values-composed.js
+++ b/packages/processor/plugins/values-composed.js
@@ -4,7 +4,7 @@ const parser = require("../parsers/parser.js");
 
 const plugin = "modular-css-values-composed";
 
-// Find @value fooga: wooga entries & catalog/remove them
+// Find @value fooga from "./wooga.css" entries & catalog/remove them
 module.exports = (css, { opts, messages }) => {
     const { files, resolve, from } = opts;
     
@@ -25,7 +25,7 @@ module.exports = (css, { opts, messages }) => {
 
         rule.remove();
     });
-    
+
     if(Object.keys(values).length > 0) {
         messages.push({
             type : "modular-css",
diff --git a/packages/svelte/svelte.js b/packages/svelte/svelte.js
index e6434950..c6d2ee1e 100644
--- a/packages/svelte/svelte.js
+++ b/packages/svelte/svelte.js
@@ -3,7 +3,6 @@
 const path = require("path");
 
 const resolve = require("resolve-from");
-const dedent = require("dedent");
 const isUrl = require("is-url");
 const escape = require("escape-string-regexp");
 
@@ -23,12 +22,41 @@ module.exports = (config = false) => {
     // eslint-disable-next-line no-console, no-empty-function
     const log = config.verbose ? console.log.bind(console, "[svelte]") : () => {};
 
+    // Check for and stringify any values in the template we couldn't convert
+    const missing = ({ source, file }) => {
+        const missed = source.match(missedRegex);
+
+        if(!missed) {
+            return source;
+        }
+         
+        const { strict } = processor.options;
+
+        const classes = missed.map((reference) => reference.replace("css.", ""));
+
+        if(strict) {
+            throw new Error(`@modular-css/svelte: Unable to find .${classes.join(", .")} in "${file}"`);
+        }
+
+        classes.forEach((key) =>
+            // eslint-disable-next-line no-console
+            console.warn(`@modular-css/svelte: Unable to find .${key} in "${file}"`)
+        );
+
+        // Turn all missing values into strings so nothing explodes
+        return source.replace(
+            new RegExp(`(${missed.map((ref) => escape(ref)).join("|")})`),
+            (match) => JSON.stringify(match)
+        );
+    };
+
     // This function is hilariously large but it's actually simpler this way
     // Mostly because markup() is async so tracking state is painful w/o inlining
     // the whole damn thing
     // eslint-disable-next-line max-statements, complexity
-    const markup = async ({ content, filename: html }) => {
+    const markup = async ({ content, filename : html }) => {
         let source = content;
+        let dependencies = [];
 
         const links = source.match(linkRegex);
         const style = source.match(styleRegex);
@@ -40,7 +68,10 @@ module.exports = (config = false) => {
         // No-op
         if(!links && !style) {
             return {
-                code : source,
+                code : missing({
+                    source,
+                    file : html,
+                }),
             };
         }
 
@@ -89,7 +120,10 @@ module.exports = (config = false) => {
             // No-op
             if(!valid.length) {
                 return {
-                    code : source,
+                    code : missing({
+                        source,
+                        file : html,
+                    }),
                 };
             }
 
@@ -117,8 +151,7 @@ module.exports = (config = false) => {
             // Remove the <link> element from the component to avoid double-loading
             source = source.replace(new RegExp(`${escape(link)}\r?\n?`), "");
 
-            // To get rollup to watch the CSS file we need to inject an import statement
-            // if a <script> block already exists hijack it otherwise inject a simple one
+            // Inject the link into the <script> block if it exists for JS referencing
             const script = source.match(scriptRegex);
 
             if(script) {
@@ -128,13 +161,9 @@ module.exports = (config = false) => {
                     tag,
                     tag.replace(contents, `\nimport css from ${JSON.stringify(css)};\n\n${contents}`)
                 );
-            } else {
-                source += dedent(`
-                    <script>
-                        import css from ${JSON.stringify(css)};
-                    </script>
-                `);
             }
+
+            dependencies = processor.dependencies(external);
         }
 
         log("processed styles", html);
@@ -171,26 +200,12 @@ module.exports = (config = false) => {
                 );
         }
 
-        // Check for any values in the template we couldn't convert
-        const missed = source.match(missedRegex);
-
-        if(missed) {
-            const { strict } = processor.options;
-
-            const classes = missed.map((reference) => reference.split("css.")[1]);
-
-            if(strict) {
-                throw new Error(`@modular-css/svelte: Unable to find .${classes.join(", .")} in "${css}"`);
-            }
-
-            classes.forEach((key) =>
-                // eslint-disable-next-line no-console
-                console.warn(`@modular-css/svelte: Unable to find .${key} in "${css}"`)
-            );
-        }
-
         return {
-            code : source,
+            code : missing({
+                source,
+                file : css
+            }),
+            dependencies,
         };
     };
 


Test patch:
diff --git a/packages/svelte/test/__snapshots__/rollup.test.js.snap b/packages/svelte/test/__snapshots__/rollup.test.js.snap
new file mode 100644
index 00000000..d73e582d
--- /dev/null
+++ b/packages/svelte/test/__snapshots__/rollup.test.js.snap
@@ -0,0 +1,32 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`/svelte.js rollup watching should generate updated output 1`] = `
+Snapshot Diff:
+- First value
++ Second value
+
+@@ -1,6 +1,10 @@
+  Object {
++   "assets/app.css": "/* packages/svelte/test/output/rollup/input/app.css */
++ .mc043115d5_nope {
++     color: blue;
++ }",
+    "output.js": "function noop() {}
+  
+  function assign(tar, src) {
+  	for (var k in src) tar[k] = src[k];
+  	return tar;
+@@ -158,11 +162,11 @@
+  
+  	return {
+  		c() {
+  			div = createElement(\\"div\\");
+  			div.textContent = \\"Hi\\";
+- 			div.className = \\"css.nope\\";
++ 			div.className = \\"mc043115d5_nope\\";
+  		},
+  
+  		m(target, anchor) {
+  			insert(target, div, anchor);
+  		},
+`;
diff --git a/packages/svelte/test/__snapshots__/svelte.test.js.snap b/packages/svelte/test/__snapshots__/svelte.test.js.snap
index 1075c404..d789357d 100644
--- a/packages/svelte/test/__snapshots__/svelte.test.js.snap
+++ b/packages/svelte/test/__snapshots__/svelte.test.js.snap
@@ -60,9 +60,7 @@ exports[`/svelte.js should extract CSS from a <link> tag (no script) 1`] = `
         <p class=\\"{bool ? \\"text\\" : \\"active\\" }\\">Text</p>
     </div>
 </div>
-<script>
-    import css from \\"./external.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should extract CSS from a <link> tag (no script) 2`] = `
@@ -95,9 +93,7 @@ exports[`/svelte.js should extract CSS from a <link> tag (single quotes) 1`] = `
         <p class=\\"{bool ? \\"text\\" : \\"active\\" }\\">Text</p>
     </div>
 </div>
-<script>
-    import css from \\"./external.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should extract CSS from a <link> tag (single quotes) 2`] = `
@@ -130,9 +126,7 @@ exports[`/svelte.js should extract CSS from a <link> tag (unquoted) 1`] = `
         <p class=\\"{bool ? \\"text\\" : \\"active\\" }\\">Text</p>
     </div>
 </div>
-<script>
-    import css from \\"./external.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should extract CSS from a <link> tag (unquoted) 2`] = `
@@ -259,11 +253,9 @@ Array [
 
 exports[`/svelte.js should handle errors: empty css file - <link> 3`] = `
 "
-<div class=\\"{css.nope}\\">NOPE</div>
+<div class=\\"{\\"css.nope\\"}\\">NOPE</div>
 <div class=\\"{css.alsonope}\\">STILL NOPE</div>
-<script>
-    import css from \\"./empty.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should handle errors: empty css file - <style> 1`] = `"@modular-css/svelte: Unable to find .nope, .alsonope in \\"<style>\\""`;
@@ -280,7 +272,7 @@ Array [
 `;
 
 exports[`/svelte.js should handle errors: empty css file - <style> 3`] = `
-"<div class=\\"{css.nope}\\">NOPE</div>
+"<div class=\\"{\\"css.nope\\"}\\">NOPE</div>
 <div class=\\"{css.alsonope}\\">STILL NOPE</div>
 
 <style>/* replaced by modular-css */</style>
@@ -305,7 +297,7 @@ exports[`/svelte.js should handle errors: invalid reference <script> - <link> 3`
 import css from \\"./invalid.css\\";
 
 
-    console.log(css.nuhuh);
+    console.log(\\"css.nuhuh\\");
 </script>
 "
 `;
@@ -326,7 +318,7 @@ exports[`/svelte.js should handle errors: invalid reference <script> - <style> 3
 <style>/* replaced by modular-css */</style>
 
 <script>
-    console.log(css.nuhuh);
+    console.log(\\"css.nuhuh\\");
 </script>
 "
 `;
@@ -343,11 +335,9 @@ Array [
 
 exports[`/svelte.js should handle errors: invalid reference template - <link> 3`] = `
 "
-<h1 class=\\"{css.nope}\\">Nope</h1>
+<h1 class=\\"{\\"css.nope\\"}\\">Nope</h1>
 <h2 class=\\"yup\\">Yup</h2>
-<script>
-    import css from \\"./invalid.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should handle errors: invalid reference template - <style> 1`] = `"@modular-css/svelte: Unable to find .nope in \\"<style>\\""`;
@@ -361,7 +351,7 @@ Array [
 `;
 
 exports[`/svelte.js should handle errors: invalid reference template - <style> 3`] = `
-"<h1 class=\\"{css.nope}\\">Nope</h1>
+"<h1 class=\\"{\\"css.nope\\"}\\">Nope</h1>
 <h2 class=\\"yup\\">Yup</h2>
 
 <style>/* replaced by modular-css */</style>
@@ -372,9 +362,7 @@ exports[`/svelte.js should ignore <links> that reference a URL 1`] = `
 "<link rel=\\"stylesheet\\" href=\\"http://example.com/styles.css\\" />
 
 <div class=\\"fooga\\">fooga</div>
-<script>
-    import css from \\"./simple.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should ignore <links> that reference a URL 2`] = `
@@ -392,11 +380,7 @@ exports[`/svelte.js should ignore files without <style> blocks 1`] = `
 
 exports[`/svelte.js should ignore files without <style> blocks 2`] = `""`;
 
-exports[`/svelte.js should invalidate files before reprocessing (<link>) 1`] = `
-"<div class=\\"source\\">Source</div><script>
-    import css from \\"./source.css\\";
-</script>"
-`;
+exports[`/svelte.js should invalidate files before reprocessing (<link>) 1`] = `"<div class=\\"source\\">Source</div>"`;
 
 exports[`/svelte.js should invalidate files before reprocessing (<link>) 2`] = `
 "/* packages/svelte/test/output/source.css */
@@ -405,11 +389,7 @@ exports[`/svelte.js should invalidate files before reprocessing (<link>) 2`] = `
 }"
 `;
 
-exports[`/svelte.js should invalidate files before reprocessing (<link>) 3`] = `
-"<div class=\\"source\\">Source</div><script>
-    import css from \\"./source.css\\";
-</script>"
-`;
+exports[`/svelte.js should invalidate files before reprocessing (<link>) 3`] = `"<div class=\\"source\\">Source</div>"`;
 
 exports[`/svelte.js should invalidate files before reprocessing (<link>) 4`] = `
 "/* packages/svelte/test/output/source.css */
@@ -660,9 +640,7 @@ exports[`/svelte.js should use an already-created processor 1`] = `
 "<link rel=\\"stylesheet\\" href=\\"http://example.com/styles.css\\" />
 
 <div class=\\"fooga\\">fooga</div>
-<script>
-    import css from \\"./simple.css\\";
-</script>"
+"
 `;
 
 exports[`/svelte.js should use an already-created processor 2`] = `
@@ -689,10 +667,8 @@ exports[`/svelte.js should warn when multiple <link> elements are in the html 1`
 "<link rel=\\"stylesheet\\" href=\\"./simple2.css\\" />
 
 <div class=\\"fooga\\">fooga</div>
-<div class=\\"{css.booga}\\">booga</div>
-<script>
-    import css from \\"./simple.css\\";
-</script>"
+<div class=\\"{\\"css.booga\\"}\\">booga</div>
+"
 `;
 
 exports[`/svelte.js should warn when multiple <link> elements are in the html 2`] = `
diff --git a/packages/svelte/test/rollup.test.js b/packages/svelte/test/rollup.test.js
new file mode 100644
index 00000000..31eeba3f
--- /dev/null
+++ b/packages/svelte/test/rollup.test.js
@@ -0,0 +1,88 @@
+/* eslint consistent-return: off */
+"use strict";
+
+const dedent = require("dedent");
+const shell = require("shelljs");
+
+const write = require("@modular-css/test-utils/write.js")(__dirname);
+const prefix = require("@modular-css/test-utils/prefix.js")(__dirname);
+const dir = require("@modular-css/test-utils/read-dir.js")(__dirname);
+const watching = require("@modular-css/test-utils/rollup-watching.js");
+
+const plugin = require("../svelte.js");
+
+const assetFileNames = "assets/[name][extname]";
+const format = "es";
+
+describe("/svelte.js", () => {
+    describe("rollup watching", () => {
+        const { watch } = require("rollup");
+        let watcher;
+        
+        beforeAll(() => shell.rm("-rf", prefix(`./output/rollup/*`)));
+        afterEach(() => watcher.close());
+        
+        it("should generate updated output", (done) => {
+            const { preprocess, processor } = plugin();
+
+            let v1;
+            let v2;
+            
+            // Create v1 of the files
+            write(`./rollup/input/index.js`, dedent(`
+                import app from "./app.html";
+                console.log(app);
+            `));
+
+            write(`./rollup/input/app.html`, dedent(`
+                <div class="{css.nope}">Hi</div>
+            `));
+
+            // Start watching
+            watcher = watch({
+                input  : prefix(`./output/rollup/input/index.js`),
+                output : {
+                    file : prefix(`./output/rollup/output/output.js`),
+                    format,
+                    assetFileNames,
+                },
+                plugins : [
+                    require("rollup-plugin-svelte")({
+                        preprocess,
+                    }),
+                    require("@modular-css/rollup")({
+                        processor,
+                    }),
+                ],
+            });
+
+            watcher.on("event", watching((builds) => {
+                if(builds === 1) {
+                    v1 = dir("./rollup/output/");
+
+                    setTimeout(() => {
+                        write(`./rollup/input/app.css`, dedent(`
+                            .nope {
+                                color: blue;
+                            }
+                        `));
+
+                        write(`./rollup/input/app.html`, dedent(`
+                            <link rel="stylesheet" href="./app.css" />
+                            <div class="{css.nope}">Hi</div>
+                        `));
+                    }, 100);
+
+                    // continue watching
+                    return;
+                }
+
+                v2 = dir("./rollup/output/");
+
+                expect(v1).toMatchDiffSnapshot(v2);
+
+                return done();
+            }));
+        });
+    });
+});