Skip to content

Commit

Permalink
Fix getOptions() with unexpected queries
Browse files Browse the repository at this point in the history
Unify the handling of unexpected queries:
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`

#56
  • Loading branch information
jhnns committed Feb 21, 2017
1 parent 8cda6ab commit 2e70ec2
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 12 deletions.
6 changes: 5 additions & 1 deletion lib/getOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions lib/parseQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
53 changes: 42 additions & 11 deletions test/getOptions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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({
Expand All @@ -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(
Expand All @@ -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
);
});
});
});

0 comments on commit 2e70ec2

Please sign in to comment.