-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Feature Request: Make customization practical/usable #241
Comments
Thank you for your detailed description. I see your point and am happy to accept a MR. |
Hi! I am facing the same issue. I would like to customise some aspects while relying on the original behaviour for whatever stays the same. It is almost impossible without rolling a completely own logging solution keeping only the skeleton of tslog or forking it an do customisation directly there. Private methodsAs per the issue, on one hand is there any reason why so many methods on export class MyCustomisedLogger<LogObj> extends BaseLogger<LogObj> {
public _toLogObj(args: unknown[], clonedLogObj: LogObj = {} as LogObj): LogObj {
// Do some things to `args` and `clonedLogObj`
const logObj = super._toLogObj(args, clonedLogObj)
// Do some more things to `logObj`
return logObj
}
} It is not clear why it is not allowed by marking those methods private. Imho all those internal methods could be made public, this would jumpstart versatility in customising behaviour. Masked exportsThe other part is that many of the utilities used by the package are masked and cannot be accessed from outside. This - as described above - makes replacing certain behaviours a huge burden, because not only the parts we want to change need to be rewritten, but the whole feature. This drives the developers to copy-paste code from the package to their own codebase, which is very suboptimal (e.g. any issues fixed in the package now manually need to be ported to the copied code, which requires monitoring). Again, it is not clear why those methods are hidden from the external world if on purpose? But in an ideal case they would be exposed and reusable. One solution could be to add another export for them, such as: {
"name": "tslog",
"exports": {
".": {
"types": "./dist/types/index.d.ts",
"require": "./dist/cjs/index.js",
"import": "./dist/esm/index.js",
"default": "./dist/esm/index.js"
}
"./deps/*": {
"types": "./dist/types/*/*.ts",
"require": "./dist/cjs/*/*.js",
"import": "./dist/esm/*/*.js",
"default": "./dist/esm/*/*.js"
}
},
} With that in place one could simply reuse the parts in the original issue, e.g.: import { isError, isBuffer } from 'tslog/deps/runtime/nodejs/index.js' (Or at least I think, haven't tried, the export/import path mappings might need some tweaking, but I think the general idea is clear. Also I'm not sure how this would work on the browser side...?) Internal vs. Public APIOkay, fair enough, both the utilities in the additional files and the methods in But on the other hand there is the argument of this issue. So in my world there should be a way to say: relying on any of those internals is something to do at your own risk and they are not guaranteed to stay the way they are. Still anyone can pin the version they are using if that is a problem for them. As per myself, I'd rather adjust my customisations every now and then when code breaks due to updates changing methods that are not part of the official public API as opposed to having to copy and paste / rewrite huge chunks of (by the way great) generic behaviour code. Because at that point I'd rather just fork the package and do the changes I need in there. The maintenance burden would already be totally with me, so it would still be much cleaner to maintain a repo and merge upstream changes every now and than. Which is fine tbh, sometimes we need to fork things. The only point here is that the library promises great customisability. The only question is what is left from the original implementation once we start to customise the way it is currently possible. Looks like not much. |
Description / Use Case for a Feature
The new version of TSLog boasts the ability to "overwrite pretty much every aspect of the log processing"; however, while technically true, it isn't practical to use. Please work on the usability of the feature.
In order to overwrite a part of the lifecycle, even for a very simple tweak, and then return to normal workflow, we have to recreate substantial portions of internal code. So many of the internal methods are either designated private or not exported, so once we attempt to overwrite anything, there doesn't seem to be any practical way to rejoin the normal workflow without a massive out-of-scope rewrite effort. The way the methods are presented in the package (whether explicitly private or hidden via the package.json), any simple overwrite seems to require an extremely in depth, recursive recreation of every dependent task that the feature being overwritten performs; this makes the overwrite feature just not worth the effort.
There should be a way to link back into default behavior and processing once we've accomplished whatever tweaks we needed with the overwrite.
Example;
If I want to overwrite
toLogObj
, to enhance it in any way, the code inBaseLogger.log
no longer calls its internal_toLogObj
method -- which is reasonable: this is what I'm overwriting. In my case, I was hoping to manipulate the file paths and convert any parenthesis to something else, to make a workaround for the bug #240. This could be a trivial task; however, I can't do it without going down a very deep rabbit hole of recreating code that has nothing to do with my very simple mod. The problem is that I'm now prevented from linking back into any default behavior._toLogObj
uses:isError
,_toErrorObject
, andisBuffer
. One of those methods is private; the other two are not accessible due to how thepackage.json
exports are configured. This problem is recursive. Therefore, if I overwrite_toLogObj
, I am now forced to also rewrite every single internal dependency that extends from that method. The task almost immediately spins out of control, becoming a massive out of scope undertaking.The text was updated successfully, but these errors were encountered: