From ef14e23bb7da8b5f4118b5002000fee0c8a2162d Mon Sep 17 00:00:00 2001
From: Pat Cavit <github@patcavit.com>
Date: Tue, 10 Jul 2018 23:56:24 -0700
Subject: [PATCH] feat: emit warnings on missing keys

Also adds strict mode which will throw errors on missing keys.
---
 docs/svelte.md                                | 10 +++
 packages/svelte/README.md                     |  6 ++
 packages/svelte/src/markup.js                 | 69 +++++++++++----
 packages/svelte/svelte.js                     |  8 +-
 .../test/__snapshots__/svelte.test.js.snap    | 86 +++++++++++++++++--
 .../specimens/invalid-external-script.html    |  7 ++
 .../specimens/invalid-external-template.html  |  4 +
 .../test/specimens/invalid-inline-script.html |  9 ++
 .../specimens/invalid-inline-template.html    |  6 ++
 packages/svelte/test/specimens/invalid.css    |  3 +
 packages/svelte/test/svelte.test.js           | 56 ++++++++----
 11 files changed, 224 insertions(+), 40 deletions(-)
 create mode 100644 packages/svelte/test/specimens/invalid-external-script.html
 create mode 100644 packages/svelte/test/specimens/invalid-external-template.html
 create mode 100644 packages/svelte/test/specimens/invalid-inline-script.html
 create mode 100644 packages/svelte/test/specimens/invalid-inline-template.html
 create mode 100644 packages/svelte/test/specimens/invalid.css

diff --git a/docs/svelte.md b/docs/svelte.md
index 6a78aa8bd..6bc47f1d6 100644
--- a/docs/svelte.md
+++ b/docs/svelte.md
@@ -89,3 +89,13 @@ module.exports = {
     ]
 };
 ```
+
+## Options
+
+### `strict`
+
+If `true` whenever a missing replacement is found like `{css.doesnotexist}` an error will be throw aborting the file processing. Defaults to `false`.
+
+### Shared Options
+
+All options are passed to the underlying `Processor` instance, see [Options](https://github.com/tivac/modular-css/blob/master/docs/api.md#options).
diff --git a/packages/svelte/README.md b/packages/svelte/README.md
index 93c667273..322835d25 100644
--- a/packages/svelte/README.md
+++ b/packages/svelte/README.md
@@ -129,4 +129,10 @@ module.exports = {
 
 ## Options
 
+### `strict`
+
+If `true` whenever a missing replacement is found like `{css.doesnotexist}` an error will be throw aborting the file processing. Defaults to `false`.
+
+### Shared Options
+
 All options are passed to the underlying `Processor` instance, see [Options](https://github.com/tivac/modular-css/blob/master/docs/api.md#options).
diff --git a/packages/svelte/src/markup.js b/packages/svelte/src/markup.js
index be5710be5..05d912a88 100644
--- a/packages/svelte/src/markup.js
+++ b/packages/svelte/src/markup.js
@@ -9,26 +9,46 @@ const slash = require("slash");
 const styleRegex = /<style[\S\s]*?>([\S\s]*?)<\/style>/im;
 const scriptRegex = /<script[\S\s]*?>([\S\s]*?)<\/script>/im;
 const linkRegex = /<link\b[^<>]*?\bhref=\s*(?:"([^"]+)"|'([^']+)'|([^>\s]+))[^>]*>/im;
+const missedRegex = /css\.\w+/gim;
 
-function updateCss({ content, result }) {
+function updateCss({ processor, content, result, filename }) {
     const exported = result.files[result.file].exports;
 
+    const code = content
+        // Replace simple {css.<key>} values first
+        .replace(
+            new RegExp(`{css\\.(${Object.keys(exported).join("|")})}`, "gm"),
+            (match, key) => exported[key].join(" ")
+        )
+        // Then any remaining bare css.<key> values
+        .replace(
+            new RegExp(`(\\b)css\\.(${Object.keys(exported).join("|")})(\\b)`, "gm"),
+            (match, prefix, key, suffix) => `${prefix}"${exported[key].join(" ")}"${suffix}`
+        );
+    
+    // Check for any values in the template we couldn't convert
+    const missed = code.match(missedRegex);
+
+    if(missed) {
+        const { strict } = processor.options;
+
+        if(strict) {
+            throw new Error(`modular-css-svelte: Unable to find "${missed.join(", ")}" in ${filename}`);
+        }
+
+        missed.forEach((key) =>
+            // eslint-disable-next-line no-console
+            console.warn(`modular-css-svelte: Unable to find "${key}" in ${filename}`)
+        );
+    }
+
     return {
-        code : content
-            // Replace simple {css.<key>} values first
-            .replace(
-                new RegExp(`{css\\.(${Object.keys(exported).join("|")})}`, "gm"),
-                (match, key) => exported[key].join(" ")
-            )
-            // Then any remaining bare css.<key> values
-            .replace(
-                new RegExp(`(\\b)css\\.(${Object.keys(exported).join("|")})(\\b)`, "gm"),
-                (match, prefix, key, suffix) => `${prefix}"${exported[key].join(" ")}"${suffix}`
-            ),
+        code,
     };
 }
 
 async function extractLink({ processor, content, filename, link }) {
+    // This looks weird, but it's to support multiple types of quotation marks
     const href = link[1] || link[2] || link[3];
     
     const external = slash(resolve(path.dirname(filename), href));
@@ -41,11 +61,13 @@ async function extractLink({ processor, content, filename, link }) {
     // 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
     if(script) {
+        const [ tag, contents ] = script;
+
         content = content.replace(
-            script[0],
-            script[0].replace(script[1], dedent(`
+            tag,
+            tag.replace(contents, dedent(`
                 import css from ${JSON.stringify(href)};
-                ${script[1]}
+                ${contents}
             `))
         );
     } else {
@@ -58,7 +80,12 @@ async function extractLink({ processor, content, filename, link }) {
 
     const result = await processor.file(external);
 
-    return updateCss({ content, result });
+    return updateCss({
+        processor,
+        content,
+        result,
+        filename : href,
+    });
 }
 
 async function extractStyle({ processor, content, filename, style }) {
@@ -67,16 +94,20 @@ async function extractStyle({ processor, content, filename, style }) {
         style[1]
     );
 
-    return updateCss({ content, result });
+    return updateCss({
+        processor,
+        content,
+        result,
+        filename : "<style>",
+    });
 }
 
-/* eslint-disable max-statements */
 module.exports = (processor) => ({ content, filename }) => {
     const link = content.match(linkRegex);
     const style = content.match(styleRegex);
 
     if(link && style) {
-        throw new Error("modular-css-svelte supports <style> OR <link>, but not both");
+        throw new Error("modular-css-svelte: use <style> OR <link>, but not both");
     }
 
     if(style) {
diff --git a/packages/svelte/svelte.js b/packages/svelte/svelte.js
index d07423eb4..b1b135dbf 100644
--- a/packages/svelte/svelte.js
+++ b/packages/svelte/svelte.js
@@ -6,7 +6,13 @@ const markup = require("./src/markup.js");
 const style = require("./src/style.js");
 
 module.exports = function(args) {
-    const processor = new Processor(args);
+    const config = Object.assign(
+        Object.create(null),
+        { strict : false },
+        args
+    );
+
+    const processor = new Processor(config);
 
     return {
         processor,
diff --git a/packages/svelte/test/__snapshots__/svelte.test.js.snap b/packages/svelte/test/__snapshots__/svelte.test.js.snap
index ededbcdf6..ad21fd6d0 100644
--- a/packages/svelte/test/__snapshots__/svelte.test.js.snap
+++ b/packages/svelte/test/__snapshots__/svelte.test.js.snap
@@ -205,6 +205,86 @@ exports[`/svelte.js should extract CSS from a <style> tag 2`] = `
 "
 `;
 
+exports[`/svelte.js should handle invalid references in "<script>" (inline: false, strict: false) 1`] = `
+Array [
+  Array [
+    "modular-css-svelte: Unable to find \\"css.nuhuh\\" in ./invalid.css",
+  ],
+]
+`;
+
+exports[`/svelte.js should handle invalid references in "<script>" (inline: false, strict: false) 2`] = `
+"
+
+<h2 class=\\"yup\\">Yup</h2>
+
+<script>import css from \\"./invalid.css\\";
+            
+console.log(css.nuhuh);</script>
+"
+`;
+
+exports[`/svelte.js should handle invalid references in "<script>" (inline: false, strict: true) 1`] = `"modular-css-svelte: Unable to find \\"css.nuhuh\\" in ./invalid.css"`;
+
+exports[`/svelte.js should handle invalid references in "<script>" (inline: true, strict: false) 1`] = `
+Array [
+  Array [
+    "modular-css-svelte: Unable to find \\"css.nuhuh\\" in <style>",
+  ],
+]
+`;
+
+exports[`/svelte.js should handle invalid references in "<script>" (inline: true, strict: false) 2`] = `
+"<h2 class=\\"yup\\">Yup</h2>
+
+<style>/* replaced by modular-css */</style>
+
+<script>
+    console.log(css.nuhuh);
+</script>
+"
+`;
+
+exports[`/svelte.js should handle invalid references in "<script>" (inline: true, strict: true) 1`] = `"modular-css-svelte: Unable to find \\"css.nuhuh\\" in <style>"`;
+
+exports[`/svelte.js should handle invalid references in "template" (inline: false, strict: false) 1`] = `
+Array [
+  Array [
+    "modular-css-svelte: Unable to find \\"css.nope\\" in ./invalid.css",
+  ],
+]
+`;
+
+exports[`/svelte.js should handle invalid references in "template" (inline: false, strict: false) 2`] = `
+"
+
+<h1 class=\\"{css.nope}\\">Nope</h1>
+<h2 class=\\"yup\\">Yup</h2>
+<script>
+    import css from \\"./invalid.css\\";
+</script>"
+`;
+
+exports[`/svelte.js should handle invalid references in "template" (inline: false, strict: true) 1`] = `"modular-css-svelte: Unable to find \\"css.nope\\" in ./invalid.css"`;
+
+exports[`/svelte.js should handle invalid references in "template" (inline: true, strict: false) 1`] = `
+Array [
+  Array [
+    "modular-css-svelte: Unable to find \\"css.nope\\" in <style>",
+  ],
+]
+`;
+
+exports[`/svelte.js should handle invalid references in "template" (inline: true, strict: false) 2`] = `
+"<h1 class=\\"{css.nope}\\">Nope</h1>
+<h2 class=\\"yup\\">Yup</h2>
+
+<style>/* replaced by modular-css */</style>
+"
+`;
+
+exports[`/svelte.js should handle invalid references in "template" (inline: true, strict: true) 1`] = `"modular-css-svelte: Unable to find \\"css.nope\\" in <style>"`;
+
 exports[`/svelte.js should ignore files without <style> blocks 1`] = `
 "<h1>Hello</h1>
 <script>console.log(\\"output\\")</script>"
@@ -212,8 +292,4 @@ exports[`/svelte.js should ignore files without <style> blocks 1`] = `
 
 exports[`/svelte.js should ignore files without <style> blocks 2`] = `""`;
 
-exports[`/svelte.js should ignore invalid {css.<key>} 1`] = `
-"<h1 class=\\"{css.nope}\\">Hello</h1>
-<h2 class=\\"yup\\">World</h2>
-<style>/* replaced by modular-css */</style>"
-`;
+exports[`/svelte.js should throw on both <style> and <link> in one file 1`] = `"modular-css-svelte: use <style> OR <link>, but not both"`;
diff --git a/packages/svelte/test/specimens/invalid-external-script.html b/packages/svelte/test/specimens/invalid-external-script.html
new file mode 100644
index 000000000..e42904b6c
--- /dev/null
+++ b/packages/svelte/test/specimens/invalid-external-script.html
@@ -0,0 +1,7 @@
+<link rel="stylesheet" href="./invalid.css" />
+
+<h2 class="{css.yup}">Yup</h2>
+
+<script>
+    console.log(css.nuhuh);
+</script>
diff --git a/packages/svelte/test/specimens/invalid-external-template.html b/packages/svelte/test/specimens/invalid-external-template.html
new file mode 100644
index 000000000..f98db962d
--- /dev/null
+++ b/packages/svelte/test/specimens/invalid-external-template.html
@@ -0,0 +1,4 @@
+<link rel="stylesheet" href="./invalid.css" />
+
+<h1 class="{css.nope}">Nope</h1>
+<h2 class="{css.yup}">Yup</h2>
diff --git a/packages/svelte/test/specimens/invalid-inline-script.html b/packages/svelte/test/specimens/invalid-inline-script.html
new file mode 100644
index 000000000..a6bb0f401
--- /dev/null
+++ b/packages/svelte/test/specimens/invalid-inline-script.html
@@ -0,0 +1,9 @@
+<h2 class="{css.yup}">Yup</h2>
+
+<style>
+    .yup { color: red; }
+</style>
+
+<script>
+    console.log(css.nuhuh);
+</script>
diff --git a/packages/svelte/test/specimens/invalid-inline-template.html b/packages/svelte/test/specimens/invalid-inline-template.html
new file mode 100644
index 000000000..364a0fe65
--- /dev/null
+++ b/packages/svelte/test/specimens/invalid-inline-template.html
@@ -0,0 +1,6 @@
+<h1 class="{css.nope}">Nope</h1>
+<h2 class="{css.yup}">Yup</h2>
+
+<style>
+    .yup { color: red; }
+</style>
diff --git a/packages/svelte/test/specimens/invalid.css b/packages/svelte/test/specimens/invalid.css
new file mode 100644
index 000000000..3c8db86d2
--- /dev/null
+++ b/packages/svelte/test/specimens/invalid.css
@@ -0,0 +1,3 @@
+.yup {
+    color: yellow;
+}
diff --git a/packages/svelte/test/svelte.test.js b/packages/svelte/test/svelte.test.js
index d8f58d473..63ad07de1 100644
--- a/packages/svelte/test/svelte.test.js
+++ b/packages/svelte/test/svelte.test.js
@@ -72,24 +72,51 @@ describe("/svelte.js", () => {
         expect(output.css).toMatchSnapshot();
     });
 
-    it("should ignore invalid {css.<key>}", async () => {
+    it.each`
+        title         | inline   | strict   | specimen
+        ${"<script>"} | ${true}  | ${true}  | ${"invalid-inline-script.html"}
+        ${"template"} | ${true}  | ${true}  | ${"invalid-inline-template.html"}
+        ${"<script>"} | ${true}  | ${false} | ${"invalid-inline-script.html"}
+        ${"template"} | ${true}  | ${false} | ${"invalid-inline-template.html"}
+        ${"<script>"} | ${false} | ${false} | ${"invalid-external-script.html"}
+        ${"template"} | ${false} | ${false} | ${"invalid-external-template.html"}
+        ${"<script>"} | ${false} | ${true}  | ${"invalid-external-script.html"}
+        ${"template"} | ${false} | ${true}  | ${"invalid-external-template.html"}
+    `("should handle invalid references in $title (inline: $inline, strict: $strict)", async ({ strict, specimen }) => {
+        const spy = jest.spyOn(global.console, "warn");
+
+        spy.mockImplementation(() => { /* NO-OP */ });
+        
+        const filename = require.resolve(`./specimens/${specimen}`);
+
         const { preprocess } = plugin({
             namer,
+            strict,
         });
+        
+        if(strict) {
+            await expect(
+                svelte.preprocess(
+                    fs.readFileSync(filename, "utf8"),
+                    Object.assign({}, preprocess, { filename })
+                )
+            ).rejects.toThrowErrorMatchingSnapshot();
+
+            return;
+        }
 
         const processed = await svelte.preprocess(
-            dedent(`
-                <h1 class="{css.nope}">Hello</h1>
-                <h2 class="{css.yup}">World</h2>
-                <style>.yup { color: red; }</style>
-            `),
-            Object.assign({}, preprocess, { filename : require.resolve("./specimens/style.html") })
+            fs.readFileSync(filename, "utf8"),
+            Object.assign({}, preprocess, { filename })
         );
         
+        expect(spy).toHaveBeenCalled();
+        expect(spy.mock.calls).toMatchSnapshot();
+
         expect(processed.toString()).toMatchSnapshot();
     });
 
-    it("should throw on both <style> and <link> in one file", () => {
+    it("should throw on both <style> and <link> in one file", async () => {
         const { preprocess } = plugin({
             css : "./packages/svelte/test/output/svelte.css",
             namer,
@@ -97,12 +124,11 @@ describe("/svelte.js", () => {
 
         const filename = require.resolve("./specimens/both.html");
 
-        return svelte.preprocess(
-            fs.readFileSync(filename, "utf8"),
-            Object.assign({}, preprocess, { filename })
-        )
-        .catch((error) =>
-            expect(error.message).toMatch("modular-css-svelte supports <style> OR <link>, but not both")
-        );
+        await expect(
+            svelte.preprocess(
+                fs.readFileSync(filename, "utf8"),
+                Object.assign({}, preprocess, { filename })
+            )
+        ).rejects.toThrowErrorMatchingSnapshot();
     });
 });