-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement missing router functions #2393
Changes from 6 commits
f6914dd
c157683
7e61447
8e8eed6
f905837
4b19bf1
cd5ad96
5740adc
f293306
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export const Blaze = { | ||
remove() {}, | ||
renderWithData() {} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export default class BlazeComponent {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export const Meteor = { | ||
isClient: false, | ||
isServer: true | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
|
||
class Dependency { | ||
depend() {} | ||
changed() {} | ||
} | ||
|
||
export const Tracker = { | ||
Dependency | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
jest.mock("/lib/collections", () => { | ||
return { | ||
Packages: {}, | ||
Shops: {} | ||
}; | ||
}); | ||
|
||
import Router from "../router.js"; | ||
|
||
|
||
/** | ||
* Order Summary is a display only component | ||
* It receives props and displays them accordingly | ||
*/ | ||
|
||
beforeEach(() => { | ||
// Set some default routes | ||
Router.setCurrentRoute({}); | ||
Router.routes = [ | ||
{ | ||
fullPath: "/test?filter=red", | ||
query: { | ||
filter: "red" | ||
}, | ||
route: { | ||
name: "test", | ||
path: "/test", | ||
route: "/test" | ||
} | ||
}, | ||
{ | ||
params: { | ||
id: "a12345" | ||
}, | ||
query: { | ||
filter: "green" | ||
}, | ||
route: { | ||
fullPath: "/reaction/test/1234?filter=green", | ||
group: { | ||
prefix: "/reaction" | ||
}, | ||
name: "singleTestItem", | ||
path: "/reaction/test/12345", | ||
route: "/reaction/test/:id" | ||
} | ||
} | ||
]; | ||
}); | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
|
||
test("Route name to equal test", () => { | ||
// TODO: Update test when server router is implemented to use Router.go() | ||
Router.setCurrentRoute(Router.routes[0]); | ||
|
||
expect(Router.getRouteName()).toBe("test"); | ||
}); | ||
|
||
test("Route query param 'filter' to equal 'red'", () => { | ||
// TODO: Update test when server router is implemented to use Router.go() | ||
Router.setCurrentRoute(Router.routes[0]); | ||
|
||
const queryValue = Router.getQueryParam("filter"); | ||
|
||
expect(queryValue).toBe("red"); | ||
}); | ||
|
||
test("Route /test/:id param 'id' to equal '12345'", () => { | ||
// TODO: Update test when server router is implemented to use Router.go() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this comment relevant to every test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's relevant to every test. I can deduplicate if it's an issue. |
||
Router.setCurrentRoute(Router.routes[1]); | ||
|
||
const received = Router.getParam("id"); | ||
|
||
expect(received).toBe("a12345"); | ||
}); | ||
|
||
test("Route is active with name 'singleTestItem'", () => { | ||
// TODO: Update test when server router is implemented to use Router.go() | ||
Router.setCurrentRoute(Router.routes[1]); | ||
|
||
const received = Router.isActiveClassName("singleTestItem"); | ||
|
||
expect(received).toBe("active"); | ||
}); | ||
|
||
test("Route /test/:id is active with prefixed path '/reaction/test/12345'", () => { | ||
// TODO: Update test when server router is implemented to use Router.go() | ||
Router.setCurrentRoute(Router.routes[1]); | ||
|
||
const received = Router.isActiveClassName("/reaction/test/12345"); | ||
|
||
expect(received).toBe("active"); | ||
}); | ||
|
||
test("Route /test is active with un-prefixed path '/test'", () => { | ||
// TODO: Update test when server router is implemented to use Router.go() | ||
Router.setCurrentRoute(Router.routes[0]); | ||
|
||
const received = Router.isActiveClassName("/test"); | ||
|
||
expect(received).toBe("active"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ class Router { | |
static Hooks = Hooks | ||
static routes = [] | ||
static _initialized = false; | ||
static activeClassName = "active"; | ||
|
||
static set _routes(value) { | ||
Router.routes = value; | ||
|
@@ -70,7 +71,7 @@ class Router { | |
static getRouteName() { | ||
const current = Router.current(); | ||
|
||
return current.options && current.options.name || ""; | ||
return current.route && current.route.name || ""; | ||
} | ||
|
||
static getParam(name) { | ||
|
@@ -87,6 +88,32 @@ class Router { | |
return current.query && current.query[name] || undefined; | ||
} | ||
|
||
static setQueryParams(newParams) { | ||
const current = Router.current(); | ||
const queryParams = Object.assign({}, current.query, newParams); | ||
|
||
for (const key in queryParams) { | ||
if (queryParams[key] === null || queryParams[key] === undefined) { | ||
delete queryParams[k]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where is |
||
} | ||
} | ||
|
||
Router.go(current.route.name, current.params, queryParams); | ||
} | ||
|
||
static setQueryParams(newParams) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you intend to have two implementations of |
||
const current = Router.current(); | ||
const queryParams = Object.assign({}, current.query, newParams); | ||
|
||
for (const key in queryParams) { | ||
if (queryParams[key] === null || queryParams[key] === undefined) { | ||
delete queryParams[k]; | ||
} | ||
} | ||
|
||
Router.go(current.route.name, current.params, queryParams); | ||
} | ||
|
||
static watchPathChange() { | ||
routerChangeDependency.depend(); | ||
} | ||
|
@@ -199,18 +226,26 @@ Router.reload = () => { | |
Router.isActiveClassName = (routeName) => { | ||
const current = Router.current(); | ||
const group = current.route.group; | ||
const path = current.route.path; | ||
let prefix; | ||
let prefix = ""; | ||
|
||
if (group && group.prefix) { | ||
prefix = current.route.group.prefix; | ||
} else { | ||
prefix = ""; | ||
} | ||
if (current.route) { | ||
const path = current.route.path; | ||
|
||
if (group && group.prefix) { | ||
prefix = current.route.group.prefix; | ||
} | ||
|
||
if (typeof path === "string") { | ||
const routeDef = path.replace(prefix + "/", ""); | ||
return routeDef === routeName ? "active" : ""; | ||
// Match route | ||
if (prefix.length && routeName.startsWith(prefix) && path === routeName) { | ||
// Route name is a path and starts with the prefix. (default '/reaction') | ||
return Router.activeClassName; | ||
} else if (routeName.startsWith("/") && path === routeName) { | ||
// Route name isa path and starts with slash, but was not prefixed | ||
return Router.activeClassName; | ||
} else if (current.route.name === routeName) { | ||
// Route name is the actual name of the route | ||
return Router.activeClassName; | ||
} | ||
} | ||
|
||
return ""; | ||
|
@@ -236,7 +271,7 @@ function hasRoutePermission(route) { | |
|
||
|
||
/** | ||
* getRouteName | ||
* getRegistryRouteName | ||
* assemble route name to be standard | ||
* prefix/package name + registry name or route | ||
* @param {String} packageName [package name] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these copy/paste artifacts?