logform: [Bug]: 2.5.0 Property 'timestamp' and 'stack does not exist on type 'TransformableInfo'.

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.

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.

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) }));
                                                                                              ~~~~~~~~~

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.

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.

@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 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.