[Bug]: 2.5.0 Property 'timestamp' and 'stack does not exist on type 'TransformableInfo'.
heliovasco opened this issue · 24 comments
The problem
Property 'timestamp' does not exist on type 'TransformableInfo'.
Property 'stack' does not exist on type 'TransformableInfo'.
What version of Logform presents the issue?
v2.5.0
What version of Node are you using?
v14.15.4
If this worked in a previous version of Logform, which was it?
v2.4.2
Minimum Working Example
transports: [ new transports.Console({ format: format.combine( format.colorize(), format.printf((nfo) => { const message = typeof nfo.message === "object" ? JSON.stringify(nfo.message) : nfo.message; return [nfo.timestamp, nfo.level, message, nfo.stack].filter(Boolean).join(" | "); }), ), handleExceptions: true, level: "debug", }), ],
Additional information
No response
🔎 Search Terms
TransformableInfo
Our build is also failing due to same issue. We depend on winston package, which indirectly depends upon this.
Same failing here, at least for now we can specify:
"overrides": {
"logform": "2.4.2"
}
in our package.json (using overrides requires npm 8.3+).
Are you still seeing this issue with logform 2.5.1?
On the surface, it doesn't look like a duplicate of #242, but worth checking.
Using logform 2.5.1 still breaks us:
> tsc
node_modules/logform/index.d.ts:14:4 - error TS1023: An index signature parameter type must be either 'string' or 'number'.
14 [key: string | symbol]: any;
~~~
src/modules/logger.ts:4:93 - error TS2339: Property 'timestamp' does not exist on type 'TransformableInfo'.
4 transports.push(new winston.transports.Console({ format: winston.format.printf(info => info.timestamp + " " + info.message) }));
~~~~~~~~~
+1 just now getting this issue as a result of 2.5.1
This issue comes from #240 but seems OK in TS 4.4 (beta)+. It looks like this TS PR added support.
What version of TypeScript are you running?
4.3.4 for me
Can you try using a later version of TypeScript? This was added to TS in summer of 2021.
Works, but honestly -- making devs upgrade their biggest dep (typescript) which has cascading effects is a really poor solution to a build breaking issue with your very tiny library. Lost an hour today over this.
If you're going to insist on keeping to old versions, pinning at 2.4.2 as suggested above is also an option.
Check out the comment on #240 for the other side of the argument.
Submitting a PR with tests to check for regressions like this is also an option to help increase the odds that this dependency doesn't break, if anyone reading this wants to help out. There is currently zero budget supporting this project directly.
In general, this repo should be using our semantic versioning better than we are here. It makes total sense to deprecate TS<4.4 support, but make sure that's a major version bump, not a minor version bump so dependencies using the ^ don't pick up that change.
In general, this repo should be using our semantic versioning better than we are here. It makes total sense to deprecate TS<4.4 support, but make sure that's a major version bump, not a minor version bump so dependencies using the ^ don't pick up that change.
Came here to say this as well.
Generally speaking, I agree.
However, TS support isn't even part of this project's testing suite and I'm not sure how many old versions it's reasonable to expect extremely part-time volunteer maintainers to manually check for on what's described as something that's been an issue and available fix for "over two years."
This set of projects also winds up with people wanting a lot of extended support for older major versions and there's some question about how much backporting maintenance burden should be added for maintaining compatibility with what is in Typescript terms a pretty old version of that project (which isn't even listed as a dependency of this one).
The test suite doesn't even cover compilation as a dependency with the current version of TS, which (a) it really should and (b) is how we got #242 from the same release. However, setting that up is a nontrivial effort and it does not appear that anybody who would benefit from that effort is willing to either do it or incentivize someone else to.
I totally understand that not being part of the test suite etc. I think the bigger issue is that this fix breaks older versions and forces an upgrade. I think the changes are great as long as they are published under the right version.
If possible can these changes be rolled back in a new version bump, and then reintroduce the these necessary changes in the proper semantic version bump?
I am taking dependency on WinstonJS which is currently set to pull the latest of logform in which is what breaks builds in some of my older projects.
Open to other suggestions as well.
Are you seeing Winston broken with 2.5.1? If so, that's a problem we should fix ASAP.
Here's the dependency chain on why this breaks other dependencies for me specifically.
An older project I am maintaining is taking dependency on Winston-Graylog2
This project is taking dependency on winston and is set to take it's latest minor and patch versions. Which is where it's now pulling in the most recent changes from this library as well due to accepting minor and patch.
So to summarize, I am two packages removed from where the problem is at on an open source that's no longer maintained. I have plans to update to later versions of all of these things, but it's not ideal to become a must fix now type issue.
Issue being that code verification tools see that dependencies are missing.
@joshd-7 Does this override strategy from earlier in the thread solve your issue?
I totally understand that not being part of the test suite etc. I think the bigger issue is that this fix breaks older versions
These two are connected: because it's not part of the test suite, it wasn't readily discoverable prior to publish that the fix breaks older versions, and the PR comments indicated the contrary (also, on independent review, it looked like a fine change that is valid in TypeScript).
There is some difficulty in the rollback strategy you suggest as that also breaks semver: anybody using 2.5.1 with this new feature will see a patch-level bump that is a breaking change removing the feature, and it would be intentional this time. If we had been able to catch the issue much earlier, especially prior to release, this would be much easier to address, but those waiting on the feature have already been notified of it being included in a 2.5 release.
In this case, the breakage only affects people using a pretty old version of TypeScript, and that audience also seems likely to be the most receptive to using this override strategy to pin to an older version of logform. Though I'm not thrilled about requiring old-TS users to take any action (either upgrade TS or add that override), this also seems like a situation where "two wrongs don't make a right." Intentionally violating semver with a change that intentionally breaks things for some people on a patch-level release seems like a worse offense than the original.
It's also worth noting that TypeScript doesn't even attempt to use semver at all, it just looks like they do and is assumed by npm (Typescript's primary recommended distribution method) in automatic upgrade/downgrade decisions. Therefore, it doesn't even make much sense to try to talk about strict adherence to semver in relation to compatibility with TypeScript.
TypeScript has also changed a lot in the last year and a half and is still relatively immature with a LOT of bugs and internal inconsistencies to work out; one might hope for a lot more bugfixes in the next year and a half (though they're not especially welcoming of outsider improvement attempts, even just at the level of identifying the issues with examples). Perhaps because of this, it also doesn't have the "long term support" type of releases that would serve as a guide for which old versions to test against. It seems the TS team expects developers to be keeping up with the latest-release version.
In the future, we could just avoid any updates for anything having to do with types (unless supported in TS <= 3.3.3333) until such time as we're ready to take on the extra maintenance burden of a major-version number increment, and call on folks like those who have posted/supported comments above to respond to comments complaining about having to keep copying over the same patches for multiple years. I'm not convinced that would be the best solution, but it would be the most compliant with semver.
Adding tests would be a better solution, making it easier to discover these types of issues sooner, but if nobody's willing to put in the work or funding for that, I don't see that solution happening anytime soon.
I'd just like to put this into context. This package has a hard dependency on triple-beam for its implementation. This project also provides a typings file. Triple-beam changed their typings two and a half years ago (DefinitelyTyped/DefinitelyTyped#46490), breaking functionality with custom winston loggers that engage at all with triple-beam. The issue was reported > 2 years ago and ignored up until two weeks ago. Dependent libraries had to copy/paste the same patch into each project using winston:
import { SPLAT } from 'triple-beam'
// Workaround until fixed.
// https://github.com/winstonjs/logform/issues/111
declare module 'logform' {
export interface TransformableInfo {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[SPLAT]: any
}
}triple-beam is also a project provided by winston. If it changes its types in an incompatible way, the changes should flow back to the dependent repos. The fact that there is now a bug trying to remediate the problem 2+ years after it was introduced is really just a symptom of the actual problem.
The fix was very likely submitted in good faith and tested with a supported version of typescript. There is no compatibility matrix, and you can't expect open source contributors to run back and test every older version of typescript. Even still, there is not a really good precedent for js libs bumping major versions for typing changes. What actually should have happened is the triple-beam PR I linked above should have been rejected.
The fact that this discussion is even happening is a good thing, and I'd like to personally thank @wbt for his efforts to maintain this library. There has been radio silence here for far too long.
Yeah, that is a problem with DefinitelyTyped, un-reviewed PRs can be merged -- that's nice because some repos are not maintained and changes still need to happen, but the flipside is that buggy PRs can be merged and then not noticed or fixed for a long time. We could have triple-beam typings included in triple-beam directly rather than using DefinitelyTyped, but it seems like it should be such a static thing (triple-beam and its typings) that we shouldn't have to worry about changes too often, and using DT should be fine. Obviously, in this case, that was not the case. Help is welcomed to try to open appropriate PRs (now / in the future).
Just caught up on this thread and wanted to give sincere thanks to @wbt for a thorough, exemplary investigation and resolution of an issue. With a project like winston that has no maintenance budget, we have to encourage moving forward (upgrading dependencies, etc.) when possible -- even at the risk of causing issues like this. Otherwise the codebase rots. Very grateful for community feedback, e.g. about ^^this apparent workaround that's been circulated around by users, and highly encourage the community to continue engaging and tell us what to do (especially if that comes in the form of PRs or more tests!).
I am also of the opinion that we shouldn't support TS versions that are 2+ years old. For Node itself, winston maintains great compatibility, but TS is a different beast and it's likely going to be too hard for us to promise such extensive compatibility. That being said, a PR that adds a readme note about compatibility (or some very lightweight "compatibility matrix") would be welcomed. This issue might be a great motivation for someone to make such a PR.
Huge thanks again to @wbt for being such a thoughtful steward of winston, and thanks to folks like @dscalzi and @joshd-7 for the constructive discussion.
Thanks also to @DABH for all your work in getting this stood up in the first place!
I am also of the opinion that we shouldn't support TS versions that are 2+ years old. For Node itself, winston maintains great compatibility, but TS is a different beast and it's likely going to be too hard for us to promise such extensive compatibility. That being said, a PR that adds a readme note about compatibility (or some very lightweight "compatibility matrix") would be welcomed. This issue might be a great motivation for someone to make such a PR.
This case was about a TS version only about 1.5 years old. However, without a PR that adds tests against whatever 4.4+ version of TS we'd want to continue supporting, I'd be hesitant to merge a readme note that claims any more than a vague "intention to remain compatible with current/recent releases of TS" (where "/recent" is more of a hedge to not have to scramble to be compatible with the latest breaking changes in TS).
I did commit a note into the 2.5.0 changelog and release notes about this particular compatibility issue. (However, I recognize the readme may have higher visibility).
All this discussion got me thinking: how would one test against various versions of Typescript.
I came across this post: https://dev.to/joshx/test-your-npm-package-against-multiple-versions-of-its-peer-dependency-34j4 which hints at a possible approach.
I'm happy to explore this and offer a PR in the coming days, if I can get it working.
Definitely not opposed, if it’s easy to add some TS versions to our CI test matrix! (And importantly — if it’s easy / basically no work to maintain going forwards.) Thanks for investigating!
Also, if testing multiple versions is hard, even a PR that tests compilation as a dependency using one current version would be a good step forward, to help prevent future instances of #242!
I've created a PR which will resolve the issue.