Merge pull request #68 from webpack/fix/get-options

Fix/get options

Author: jhnns

README.md

@@ -11,11 +11,21 @@ Recommended way to retrieve the options of a loader invocation:
 const options = loaderUtils.getOptions(this);
 ```
 
+1. If `this.query` is a string:
+	- Tries to parse the query string and returns a new object
+	- Throws if it's not a valid query string
+2. If `this.query` is object-like, it just returns `this.query`
+3. In any other case, it just returns `null`
+
 **Please note:** The returned `options` object is *read-only*. It may be re-used across multiple invocations.
 If you pass it on to another library, make sure to make a *deep copy* of it:
 
 ```javascript
-const options = Object.assign({}, loaderUtils.getOptions(this));
+const options = Object.assign(
+	{},
+	loaderUtils.getOptions(this), // it is safe to pass null to Object.assign()
+	defaultOptions
+);
 // don't forget nested objects or arrays
 options.obj = Object.assign({}, options.obj); 
 options.arr = options.arr.slice();
@@ -29,13 +39,13 @@ someLibrary(options);
 If the loader options have been passed as loader query string (`loader?some&params`), the string is parsed like this:
 
 ``` text
-null                         -> {}
+                             -> null
 ?                            -> {}
 ?flag                        -> { flag: true }
 ?+flag                       -> { flag: true }
 ?-flag                       -> { flag: false }
 ?xyz=test                    -> { xyz: "test" }
-?xyz=1                       -> { xyz: "1" }
+?xyz=1                       -> { xyz: "1" } // numbers are NOT parsed
 ?xyz[]=a                     -> { xyz: ["a"] }
 ?flag1&flag2                 -> { flag1: true, flag2: true }
 ?+flag1,-flag2               -> { flag1: true, flag2: false }

lib/getOptions.js

@@ -4,9 +4,13 @@ const parseQuery = require("./parseQuery");
 
 function getOptions(loaderContext) {
 	const query = loaderContext.query;
-	if(typeof query === "string") {
+	if(typeof query === "string" && query !== "") {
 		return parseQuery(loaderContext.query);
 	}
+	if(!query || typeof query !== "object") {
+		// Not object-like queries are not supported.
+		return null;
+	}
 	return query;
 }
 

lib/parseQuery.js

@@ -13,6 +13,9 @@ function parseQuery(query) {
 		throw new Error("A valid query string passed to parseQuery should begin with '?'");
 	}
 	query = query.substr(1);
+	if(!query) {
+		return {};
+	}
 	if(query.substr(0, 1) === "{" && query.substr(-1) === "}") {
 		return JSON5.parse(query);
 	}

test/getOptions.test.js

@@ -4,8 +4,13 @@ const assert = require("assert");
 const loaderUtils = require("../lib");
 
 describe("getOptions()", () => {
-	describe("when loaderContext.query is a string", () => {
+	describe("when loaderContext.query is a query string starting with ?", () => {
 		[{
+			it: "should return an empty object by default",
+			query: "?",
+			expected: {}
+		},
+		{
 			it: "should parse query params",
 			query: "?name=cheesecake&slices=8&delicious&warm=false",
 			expected: {
@@ -66,17 +71,27 @@ describe("getOptions()", () => {
 				);
 			});
 		});
-		describe("and the query string does not start with ?", () => {
-			it("should throw an error", () => {
-				assert.throws(
-					() => loaderUtils.getOptions({ query: "a" }),
-					"A valid query string passed to parseQuery should begin with '?'"
-				);
-			});
+	});
+	describe("when loaderContext.query is an empty string", () => {
+		it("should return null", () => {
+			assert.strictEqual(
+				loaderUtils.getOptions({
+					query: ""
+				}),
+				null
+			);
+		});
+	});
+	describe("when loaderContext.query is any other string not starting with ?", () => {
+		it("should throw an error", () => {
+			assert.throws(
+				() => loaderUtils.getOptions({ query: "a" }),
+				"A valid query string passed to parseQuery should begin with '?'"
+			);
 		});
 	});
 	describe("when loaderContext.query is an object", () => {
-		it("should just return the object", () => {
+		it("should just return it", () => {
 			const query = {};
 			assert.strictEqual(
 				loaderUtils.getOptions({
@@ -86,7 +101,7 @@ describe("getOptions()", () => {
 			);
 		});
 	});
-	describe("when loaderContext.query is anything else", () => {
+	describe("when loaderContext.query is an array", () => {
 		it("should just return it", () => {
 			const query = [];
 			assert.strictEqual(
@@ -95,18 +110,34 @@ describe("getOptions()", () => {
 				}),
 				query
 			);
+		});
+	});
+	describe("when loaderContext.query is anything else", () => {
+		it("should return null", () => {
 			assert.strictEqual(
 				loaderUtils.getOptions({
 					query: undefined
 				}),
-				undefined
+				null
 			);
 			assert.strictEqual(
 				loaderUtils.getOptions({
 					query: null
 				}),
 				null
 			);
+			assert.strictEqual(
+				loaderUtils.getOptions({
+					query: 1
+				}),
+				null
+			);
+			assert.strictEqual(
+				loaderUtils.getOptions({
+					query: 0
+				}),
+				null
+			);
 		});
 	});
 });