-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Bug]: 'level' parameter of winston.createLogger() no longer accepts type 'string', breaks ability to set log level via environment variable #2047
Comments
This looks like potentially an unintended breaking change but arguably stronger typings are enforced now? What do you think is the best solution? Add some warning to the CHANGELOG? Make the type level | undefined | string? |
@DABH - Agreed, the stronger enforced typging is great, but it's unfortunatly blocking automated pipelines. Does the project consider this a breaking change and therefore something to be included in a subsequent major version bump? That said, certainly wouldn't want to slow down progress 😅 Your suggestion of making the type (Thanks for raising this issue, @bradenmitchell) |
What is more: using custom levels is now broken at compile time (TypeScript): const levels = {
foobar: 99,
warn: 2,
error: 1,
};
const logger = createLogger({
levels,
});
const level: keyof typeof levels = 'foobar';
logger.log(level, 'message'); // Argument of type '"foobar"' is not assignable to parameter of type 'level'.ts(2769) Due to Lines 79 to 85 in 3b21cc4
Maybe, adding generic parameter would resolve the issue. |
PR welcomed from whoever wants to implement the desired fix here! |
…m values Fixes winstonjs#2047 BREAKING CHANGE: v3.5.0 introduced typings changes that broke typings for custom (non-npm) levels Generous application of Generics allowed to circumvent the original issues, whilst maintaining typings improvements Default levels should be unaffected. Integrations using custom log levels, and the Logger interface will have to add a type argument of their interfaces.
So I opened #2048 to apply generics most everywhere to solve the overly strict types. However, for all the flexibility & ease of use I've worked into that one, it is still not globally without breaking changes. Going forward, I would strongly advise reverting #1896 and publishing it as 3.5.1. #1896 & #2048 should be released as part of a new major version, if only because they can break TypeScript code using Winston. |
Why aren't the syslog levels included in the stronger typed |
This has been done. Apologies for missing the impact of adding those types on custom levels! |
AFAIK, #2048 should also provide some typings validation for custom types using winston |
The problem
When using environment variables to set 'level' property of
winston.createLogger({ level: ... })
, VSCode displays the following error:Type 'string' is not assignable to type 'level | undefined'
.It appears that in v3.5.0 the type for the level parameter was changes from
'string | undefined'
(as in v3.4.0 and prior) to'level | undefined'
where 'level' is a type defined by winston astype level = "debug" | "error" | "warn" | "info" | "http" | "verbose" | "silly"
Even when defining the type for the environment variable to
winston.level
this type error still occurs:then
Current workaround
import level and assert the environment variable as the level type.
What version of Winston presents the issue?
3.5.0
What version of Node are you using?
14.17.3
If this worked in a previous version of Winston, which was it?
3.4.0
Minimum Working Example
No response
Additional information
No response
The text was updated successfully, but these errors were encountered: