-
Notifications
You must be signed in to change notification settings - Fork 86
feat(routes): childRoutes are not required for root module #657
feat(routes): childRoutes are not required for root module #657
Conversation
📊 Bundle Size Report
|
…o feat/childRoutes-not-required-in-root-module
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.
Approving the code changes, but we should document this functionality
I will do it. thanks. |
748360a
@@ -0,0 +1,3 @@ | |||
const hasChildRoutes = (module) => (module !== null && typeof module === 'object') && !!module.childRoutes; |
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.
This file needs the license header
@@ -0,0 +1,11 @@ | |||
import hasChildRoutes from '../../../src/universal/utils/hasChildRoutes'; |
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.
License header is also needed here
docs/api/modules/Routing.md
Outdated
@@ -39,7 +39,7 @@ Enables components and modules to define their own child routes. `childRoutes` c | |||
route, array of routes or a function which accepts the Redux store as the sole argument and returns | |||
a single route or array of routes. | |||
|
|||
One App requires the Root Module to have at least one child route defined. This can be either `<Route>` or `<ModuleRoute>` with a `path`. | |||
`childRoutes` is optional for Root Module. `childRoutes` can be either `<Route>` or `<ModuleRoute>` with a `path`. If `childRoutes` is not defined, default path `/` is added to RootRoute prop in one app. |
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.
`childRoutes` is optional for Root Module. `childRoutes` can be either `<Route>` or `<ModuleRoute>` with a `path`. If `childRoutes` is not defined, default path `/` is added to RootRoute prop in one app. | |
`childRoutes` is optional for Root Module. `childRoutes` can be either `<Route>` or `<ModuleRoute>` with a `path`. If `childRoutes` is not defined, the default path `/` is added to the root route. |
* permissions and limitations under the License. | ||
*/ | ||
|
||
const hasChildRoutes = (module) => module !== null && typeof module === 'object' && !!module.childRoutes; |
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.
as null
and undefined
are falsy this null check is redundant, also not a fan of the type check as i don't think its needed, anything that would throw a type error is considered falsy, anything else (string, int,symbol etc) will return undefined.
const hasChildRoutes = (module) => module !== null && typeof module === 'object' && !!module.childRoutes; | |
const hasChildRoutes = (module) => module && !!module.childRoutes; |
If you are desperate for a boolean
const hasChildRoutes = (module) => module !== null && typeof module === 'object' && !!module.childRoutes; | |
const hasChildRoutes = (module) => !!module && !!module.childRoutes; |
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.
@JAdshead Updated.
4a937e0
Description
Motivation and Context
Root modules are required to have childRoutes. With this change, childRoutes will be optional.
How Has This Been Tested?
Ran one-app server locally with root module, with and without child routes.
Test cases are written for new files and files changed.
Types of Changes
Checklist:
What is the Impact to Developers Using One App?
Root modules no longer requires childRoutes. If childRoutes are not provided, path is added, which will render root component.