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.

Sign up for GitHub

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

Jump to bottom

Add literal types and values for logger levels #2056

Open
wants to merge 2 commits into
base: 4.x
Choose a base branch
from
Open

Add literal types and values for logger levels #2056

wants to merge 2 commits into from

Conversation

mb-realpage
Copy link

@mb-realpage mb-realpage commented Nov 23, 2024 •
edited
Loading

Before creating a pull-request, please check https://github.com/handlebars-lang/handlebars.js/blob/master/CONTRIBUTING.md first.

Generally we like to see pull requests that

  • Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
  • Maintain the existing code style
  • Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
  • Have good commit messages
  • Have tests
  • Have the typings (types/index.d.ts) updated on every API change. If you need help, updating those, please mention that in the PR description.
  • Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
  • Currently, the 4.x-branch contains the latest version. Please target that branch in the PR.

Note: fixes #2055

@mb-realpage
Add literal type for logger.level string values
b207520 
By default, `logger.level` is set to "info," type: `string`, even though
types/index.d.ts specifies that `level` should be type: `number`. This
is confusing.

Worse, setting `logger.level` in TypeScript will not allow a string to
be used. A specific number must be provided, even though a string works
because `lookupLevel` converts the given string to the correct number.
So setting `logger.level` must be done in one of two unsatisfactory
ways:

* By using double assertion to lie to TypeScript, i.e., using a string
with `as unknown as number`. This defeats the purpose of types.
* By providing the exact index of the log level desired, which requires
reading Handlebars' logger implementation to see that:
  * `debug` = 0
  * `info` = 1
  * `warn` = 2
  * `error` = 3

  Note that these numbers are not available in the keys that are defined
  in the types (`DEBUG`, `INFO`, `WARN`, and `ERROR`), since they are
  not actually defined in the logger itself.
@mb-realpage
Add log level constants to logger
f0443c1 
The types for `DEBUG`, `INFO`, `WARN`, and `ERROR` are present in the
types `logger`, but not in the code. If they're in the types they should
be in the code as well.
@mb-realpage mb-realpage mentioned this pull request Nov 23, 2024
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@mb-realpage