fix: Correctly support named exports of @value (#399)

* fix: named exports of string values are broken Was quoting & not escaping values at all, which creates invalid JS in some cases. Now using `JSON.stringify()` for these values. WARNING: code like `@value val: "value"` will be `"\"value\""` in JS, I don't want to introduce special handling for it. * test: specimen/snapshot updates for exported @value * test: provide a few more examples * chore: update svelte deps * test: update svelte snapshot * chore: update rollup dep * test: update rollup snapshot
tivac · Feb 16, 2018 · 166bfb2 · 166bfb2
1 parent fe82167
commit 166bfb2
Show file tree

Hide file tree

Showing 12 changed files with 49 additions and 28 deletions.
diff --git a/packages/rollup/package.json b/packages/rollup/package.json
@@ -18,7 +18,7 @@
     "postcss"
   ],
   "devDependencies": {
-    "rollup": "^0.51.8",
+    "rollup": "^0.56.1",
     "test-utils": "^8.0.0"
   },
   "dependencies": {

diff --git a/packages/rollup/rollup.js b/packages/rollup/rollup.js
@@ -67,11 +67,10 @@ module.exports = function(opts) {
                 )
             )
             .then((results) => {
-                var result  = results[0],
-                    classes = output.join(result.exports),
-                    named   = Object.keys(classes),
-                    out     = [
-                        `export default ${JSON.stringify(classes, null, 4)};`
+                var result   = results[0],
+                    exported = output.join(result.exports),
+                    out      = [
+                        `export default ${JSON.stringify(exported, null, 4)};`
                     ];
 
                 // Add dependencies
@@ -88,14 +87,14 @@ module.exports = function(opts) {
                     };
                 }
 
-                named.forEach((ident) => {
+                Object.keys(exported).forEach((ident) => {
                     if(keyword.isReservedWordES6(ident) || !keyword.isIdentifierNameES6(ident)) {
                         options.onwarn(`Invalid JS identifier "${ident}", unable to export`);
 
                         return;
                     }
 
-                    out.push(`export var ${ident} = "${classes[ident]}";`);
+                    out.push(`export var ${ident} = ${JSON.stringify(exported[ident])};`);
                 });
 
                 return {

diff --git a/packages/rollup/test/__snapshots__/rollup.test.js.snap b/packages/rollup/test/__snapshots__/rollup.test.js.snap
@@ -2,6 +2,7 @@
 
 exports[`/rollup.js should allow disabling of named exports 1`] = `
 "var css = {
+    \\"str\\": \\"\\\\\\"string\\\\\\"\\",
     \\"fooga\\": \\"fooga\\"
 };
 
@@ -22,27 +23,29 @@ exports[`/rollup.js should generate CSS 1`] = `
     color: red;
 }
 
-/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIi4uL3NwZWNpbWVucy9zaW1wbGUuY3NzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLCtDQUFDO0FBQUQ7SUFDSSxXQUFXO0NBQ2QiLCJmaWxlIjoic2ltcGxlLmNzcyIsInNvdXJjZXNDb250ZW50IjpbIi5mb29nYSB7XG4gICAgY29sb3I6IHJlZDtcbn1cbiJdfQ== */"
+/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIi4uL3NwZWNpbWVucy9zaW1wbGUuY3NzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLCtDQUFDO0FBRUQ7SUFDSSxXQUFXO0NBQ2QiLCJmaWxlIjoic2ltcGxlLmNzcyIsInNvdXJjZXNDb250ZW50IjpbIkB2YWx1ZSBzdHI6IFwic3RyaW5nXCI7XG5cbi5mb29nYSB7XG4gICAgY29sb3I6IHJlZDtcbn1cbiJdfQ== */"
 `;
 
 exports[`/rollup.js should generate JSON 1`] = `
 "{
     \\"packages/rollup/test/specimens/simple.css\\": {
+        \\"str\\": \\"\\\\\\"string\\\\\\"\\",
         \\"fooga\\": \\"fooga\\"
     }
 }"
 `;
 
 exports[`/rollup.js should generate exports 1`] = `
 "var css = {
+    \\"str\\": \\"\\\\\\"string\\\\\\"\\",
     \\"fooga\\": \\"fooga\\"
 };
 
 console.log(css);
 "
 `;
 
-exports[`/rollup.js should generate external source maps 1`] = `"{\\"version\\":3,\\"sources\\":[\\"../specimens/simple.css\\"],\\"names\\":[],\\"mappings\\":\\"AAAA,+CAAC;AAAD;IACI,WAAW;CACd\\",\\"file\\":\\"simple.css\\",\\"sourcesContent\\":[\\".fooga {\\\\n    color: red;\\\\n}\\\\n\\"]}"`;
+exports[`/rollup.js should generate external source maps 1`] = `"{\\"version\\":3,\\"sources\\":[\\"../specimens/simple.css\\"],\\"names\\":[],\\"mappings\\":\\"AAAA,+CAAC;AAED;IACI,WAAW;CACd\\",\\"file\\":\\"simple.css\\",\\"sourcesContent\\":[\\"@value str: \\\\\\"string\\\\\\";\\\\n\\\\n.fooga {\\\\n    color: red;\\\\n}\\\\n\\"]}"`;
 
 exports[`/rollup.js should not output sourcemaps when they are disabled 1`] = `
 "/* packages/rollup/test/specimens/simple.css */
@@ -53,9 +56,13 @@ exports[`/rollup.js should not output sourcemaps when they are disabled 1`] = `
 `;
 
 exports[`/rollup.js should provide named exports 1`] = `
-"var a = \\"a\\";
+"var str = \\"\\\\\\"string\\\\\\"\\";
+var num = \\"10\\";
+var dim = \\"10px\\";
+var mix = \\"1px solid red\\";
+var a = \\"a\\";
 
-console.log(a);
+console.log(a, str, num, dim, mix);
 "
 `;
 
@@ -81,8 +88,8 @@ console.log(css, fooga);
 
 exports[`/rollup.js shouldn't disable sourcemap generation 1`] = `
 Object {
-  "file": null,
-  "mappings": ";;;;AAEA,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC",
+  "file": "simple.js",
+  "mappings": ";;;;;AAEA,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC",
   "names": Array [],
   "sourcesContent": Array [
     "import css from \\"./simple.css\\";

diff --git a/packages/rollup/test/specimens/named.css b/packages/rollup/test/specimens/named.css
@@ -1,3 +1,8 @@
+@value str: "string";
+@value num: 10;
+@value dim: 10px;
+@value mix: 1px solid red;
+
 .a {
     color: red;
 }
diff --git a/packages/rollup/test/specimens/named.js b/packages/rollup/test/specimens/named.js
@@ -1,3 +1,3 @@
-import {a} from "./named.css";
+import { a, str, num, dim, mix } from "./named.css";
 
-console.log(a);
+console.log(a, str, num, dim, mix);
diff --git a/packages/rollup/test/specimens/simple.css b/packages/rollup/test/specimens/simple.css
@@ -1,3 +1,5 @@
+@value str: "string";
+
 .fooga {
     color: red;
 }
diff --git a/packages/svelte/package.json b/packages/svelte/package.json
@@ -22,9 +22,9 @@
   ],
   "devDependencies": {
     "dedent": "^0.7.0",
-    "rollup": "^0.51.8",
-    "rollup-plugin-svelte": "^3.3.0",
-    "svelte": "^1.46.0"
+    "rollup": "^0.56.1",
+    "rollup-plugin-svelte": "^4.0.0",
+    "svelte": "^1.55.0"
   },
   "dependencies": {
     "mkdirp": "^0.5.1",

diff --git a/packages/svelte/test/__snapshots__/rollup.test.js.snap b/packages/svelte/test/__snapshots__/rollup.test.js.snap
@@ -193,6 +193,7 @@ var proto = {
 };
 
 
+
 function create_main_fragment(state, component) {
 	var div, h1, text_1, div_1, p, p_class_value;
 

diff --git a/packages/webpack/loader.js b/packages/webpack/loader.js
@@ -20,10 +20,9 @@ module.exports = function(source) {
 
     return processor.string(this.resourcePath, source)
         .then((result) => {
-            var classes = output.join(result.exports),
-                named   = Object.keys(classes),
-                out     = [
-                    `export default ${JSON.stringify(classes, null, 4)};`
+            var exported = output.join(result.exports),
+                out      = [
+                    `export default ${JSON.stringify(exported, null, 4)};`
                 ];
 
             processor.dependencies(this.resourcePath).forEach(this.addDependency);
@@ -35,14 +34,14 @@ module.exports = function(source) {
 
             // Warn if any of the exported CSS wasn't able to be used as a valid JS identifier
             // and exclude from the output
-            named.forEach((ident) => {
+            Object.keys(exported).forEach((ident) => {
                 if(keyword.isReservedWordES6(ident) || !keyword.isIdentifierNameES6(ident)) {
                     this.emitWarning(new Error(`Invalid JS identifier "${ident}", unable to export`));
 
                     return;
                 }
 
-                out.push(`export var ${ident} = "${classes[ident]}";`);
+                out.push(`export var ${ident} = ${JSON.stringify(exported[ident])};`);
             });
 
             return done(null, out.join("\n"));

diff --git a/packages/webpack/test/__snapshots__/webpack.test.js.snap b/packages/webpack/test/__snapshots__/webpack.test.js.snap
@@ -905,12 +905,15 @@ console.log(__WEBPACK_IMPORTED_MODULE_0__es2015_css__[\\"a\\" /* default */]);
 /***/ (function(module, __webpack_exports__, __webpack_require__) {
 
 \\"use strict\\";
+/* unused harmony export val */
 /* unused harmony export a */
 /* unused harmony export b */
 /* harmony default export */ __webpack_exports__[\\"a\\"] = ({
+    \\"val\\": \\"\\\\\\"value\\\\\\"\\",
     \\"a\\": \\"a\\",
     \\"b\\": \\"b\\"
 });
+var val = \\"\\\\\\"value\\\\\\"\\";
 var a = \\"a\\";
 var b = \\"b\\";
 
@@ -1000,20 +1003,23 @@ Object.defineProperty(__webpack_exports__, \\"__esModule\\", { value: true });
 /* harmony import */ var __WEBPACK_IMPORTED_MODULE_0__es2015_css__ = __webpack_require__(1);
 
 
-console.log(__WEBPACK_IMPORTED_MODULE_0__es2015_css__[\\"a\\"]);
+console.log(__WEBPACK_IMPORTED_MODULE_0__es2015_css__[\\"a\\"], __WEBPACK_IMPORTED_MODULE_0__es2015_css__[\\"b\\" /* val */]);
 
 
 /***/ }),
 /* 1 */
 /***/ (function(module, __webpack_exports__, __webpack_require__) {
 
 \\"use strict\\";
+/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, \\"b\\", function() { return val; });
 /* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, \\"a\\", function() { return a; });
 /* unused harmony export b */
 /* unused harmony default export */ var _unused_webpack_default_export = ({
+    \\"val\\": \\"\\\\\\"value\\\\\\"\\",
     \\"a\\": \\"a\\",
     \\"b\\": \\"b\\"
 });
+var val = \\"\\\\\\"value\\\\\\"\\";
 var a = \\"a\\";
 var b = \\"b\\";
 

diff --git a/packages/webpack/test/specimens/es2015-named.js b/packages/webpack/test/specimens/es2015-named.js
@@ -1,3 +1,3 @@
-import { a } from "./es2015.css";
+import { a, val } from "./es2015.css";
 
-console.log(a);
+console.log(a, val);
diff --git a/packages/webpack/test/specimens/es2015.css b/packages/webpack/test/specimens/es2015.css
@@ -1,2 +1,4 @@
+@value val: "value";
+
 .a { color: red; }
 .b { color: blue; }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -193,6 +193,7 @@ var proto = {
		};



		function create_main_fragment(state, component) {
		var div, h1, text_1, div_1, p, p_class_value;

Expand Down