-
Notifications
You must be signed in to change notification settings - Fork 16
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
Support for Boolean Type in Logger Option #24
Conversation
@15fathoms Thank you for taking time to make a PR and writing clear PR description. I have one question/observation, can users achieve the same result by not calling library? For example: if (!CONFIG.DISABLE_LOGGING) {
expressListRoutes(app, { prefix: '/api/v1' });
} vs expressListRoutes(app, { prefix: '/api/v1', logging: CONFIG.DISABLE_LOGGING }); I'm not totally against merging the change but I would urge users to do the former solution because it will avoid running code all together which may effect performance boot time if you have lots of routes. |
Thank you for your question. I realize now that I may have omitted the most important detail of this pull request. Personally, the aspect of your module that interests me is the returning of routes. I use it to sort some of the routes and the data that results from them. Although my project is not yet public, I want to illustrate my point. Here is an example of how I use express-list-routes: const endpoints = expressListRoutes(req.app, {
logger: false
});
endpoints.forEach((endpoint) => {
endpoint.path = endpoint.path.split('\\').join('/');
if (endpoint.path.endsWith('/')) {
endpoint.path = endpoint.path.slice(0, -1);
}
});
const filteredEndpoints = endpoints.filter((endpoint) => {
if (endpoint.method.includes('GET')) {
return collectionNames.some((collection) => endpoint.path.endsWith(collection));
}
return false;
}); It is possible to conditionally call the function and use dynamic import to load the module only when the condition is met and passing logger as false. Looking at your code, we can see that, whether logs are enabled or not, the loop is still executed. Including an empty function doesn't help with performance either. To improve performance, it might be interesting to separate the logic into multiple functions and call them either by default or based on defined options. However, looping is still necessary. My pull request is a request for long-term support for this feature for two reasons:
|
@15fathoms Ahh, it never occurred to me that returning a list of routes would be useful. But I now see how helpful it can be to have a pragmatic list of all your routes registered, especially in larger apps where Again thank you for taking the time to improve this PR. I will merge this PR as is, I will also tweak docs to highlight that use case. Btw, the only reason I returned paths was for testing, this was a happy side effect of doing that 😂 |
Version 1.2.2 is released https://www.npmjs.com/package/express-list-routes?activeTab=versions |
@labithiotis Thanks fellow dev friend ! 😁 Well, actually the happy side effect is REALLY useful ! For exemple, here's what i can return to list my routes inside my Given the URL [
{
"name": "fonts",
"model": "Font",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/fonts"
}
},
{
"name": "news",
"model": "New",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/news"
}
},
{
"name": "models",
"model": "Model",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/models"
}
},
{
"name": "images",
"model": "Image",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/uploads"
}
},
{
"name": "sections",
"model": "Section",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/sections"
}
},
{
"name": "users",
"model": "User",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/users"
}
},
{
"name": "galleries",
"model": "Gallery",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/galleries"
}
},
{
"name": "configs",
"model": "Config",
"endpoint": {
"method": [
"GET",
"POST"
],
"path": "/api/configs"
}
}
] Or given the exemple url : [
{
"_id": "667a0e01d894fcd15bf968fe",
"family": "DM Mono",
"variants": [
"300",
"300italic",
"regular",
"italic",
"500",
"500italic"
],
"subsets": [
"latin",
"latin-ext"
],
"__v": 0
}
] As you can see, it actually can help dynamically doing a lot off stuff depending on what you're working on. Thank you a lot for your time. 😊 |
@15fathoms your more than welcome to submit PRS, I will always try to respond in a timely manner. 😀 |
Description
This pull request adds support for using a boolean value in the logger option within the Options interface. This enhancement allows users to enable or disable logging functionality with a simple boolean flag (true or false) in addition to the existing capability of providing a custom logging function.
Detailed Changes
Modified
Options
Interface:logger
property in theOptions
interface now accepts either a function of type(method: string, space: string, path: string) => void
or aboolean
.Updated
expressListRoutes
Function:logger
beingtrue
,false
, or afunction
.logger
istrue
, the default loggerconsole.info
is used.logger
isfalse
, no logging occurs.logger
is a function, the provided function is used for logging.Benefits
Example Usage
XOXO ❤️