Skip to content
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

Make typings match code better #1158

Merged
merged 1 commit into from Jan 21, 2022
Merged

Conversation

glasser
Copy link
Contributor

@glasser glasser commented Jan 21, 2022

Fixes v6.4.0 regressions.

There is an npm script npm run typings which actually failed before
this commit. This seems to have been a combination of incorrect typings
(a mandatory argument to getLevel that should be optional, the idea
that for custom appenders configure should return an intermediate
"generator" function before the appender function itself) as well as
mistakes in the test file (which had an appender module which had one
level of function nesting too few).

Specifically, for the configure function on appenders, I believe there
should be two levels of function: an outer function taking one-time
configuration, and an inner function taking an individual log messages.
The typings file wanted there to be three layers, and the test file
wanted there to be one layer.

Fixes #1155.

Fixes v6.4.0 regressions.

There is an npm script `npm run typings` which actually failed before
this commit. This seems to have been a combination of incorrect typings
(a mandatory argument to `getLevel` that should be optional, the idea
that for custom appenders `configure` should return an intermediate
"generator" function before the appender function itself) as well as
mistakes in the test file (which had an appender module which had one
level of function nesting too few).

Specifically, for the `configure` function on appenders, I believe there
should be two levels of function: an outer function taking one-time
configuration, and an inner function taking an individual log messages.
The typings file wanted there to be three layers, and the test file
wanted there to be one layer.

Fixes log4js-node#1155.
@nomiddlename
Copy link
Collaborator

Thanks for this fix - I've added npm run typings to our PR build checks, so we should at least know if future changes are going to break the typescript tests.

@glasser
Copy link
Contributor Author

glasser commented Jan 22, 2022

Thanks! Will you be able to publish this soon as v6.4.1?

@lamweili
Copy link
Contributor

Published log4js@6.4.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong TS types in 6.4.0?
4 participants