feat!: change customPrettifiers to a Map #552
Conversation
The fourth parameter of the 'level' custom prettifier -- extras -- used
to have a separate type as pino-pretty offered two additional attributes
to this particular prettifier.
However since it used a Record<string, T> type, TypeScript required all
values to be compatible with each other. So when users tried to use the
extras parameter for 'level', it caused
error TS2322: Type '{ level: (_level: string | object, _levelKey: string, _log: object, { labelColorized }: PrettifierExtras<LevelPrettifierExtras>) => string; }' is not assignable to type 'Record<string, Prettifier<object>> & { level?: Prettifier<LevelPrettifierExtras> | undefined; }'.
Type '{ level: (_level: string | object, _levelKey: string, _log: object, { labelColorized }: PrettifierExtras<LevelPrettifierExtras>) => string; }' is not assignable to type 'Record<string, Prettifier<object>>'.
Property 'level' is incompatible with index signature.
Type '(_level: string | object, _levelKey: string, _log: object, { labelColorized }: PrettifierExtras<LevelPrettifierExtras>) => string' is not assignable to type 'Prettifier<object>'.
Types of parameters '__3' and 'extras' are incompatible.
Type 'PrettifierExtras<object>' is not assignable to type 'PrettifierExtras<LevelPrettifierExtras>'.
Type '{ colors: Colorette; }' is missing the following properties from type 'LevelPrettifierExtras': label, labelColorized
115 customPrettifiers: {
~~~~~~~~~~~~~~~~~
This patch remedies this issue by merging LevelPrettifierExtras into
PrettifierExtras. These types are not exported directly, therefore users,
including those who leverage TypeScript utility types to extract these
types, should be able to upgrade directly.
Fixes pinojs#550
Frederick888
changed the title
In [1], LevelPrettifierExtras and PrettifierExtras were merged to solve a typing issue [2]. This was suboptimal as the additional extra attributes that only the 'level' prettifier had were exposed to other prettifiers too. To allow the additional attributes while keeping 'level' prettifier's type separate, this patch uses an intersection Map type CustomPrettifiers instead. This is a breaking change. [1] pinojs#551 [2] pinojs#550
Frederick888
force-pushed
the
custom-prettifiers-map
branch
from
158698a to
38cdaf2
Compare
|
I'm not clear. What is this solving? It seems to just be making the code more complicated. |
|
@jsumners Indeed it can be a little confusing as on the JavaScript side, not only does it solve nothing, it also introduces a breaking change. The issue actually came only from the TypeScript side. Allow me to share some context. In #551 to solve #550, 
This was obviously not the intention of the original So now on this PR: If we don't want to break existing JavaScript users, I can instead allow In Please let me know what you think :) |
In [1], LevelPrettifierExtras and PrettifierExtras were merged to solve
a typing issue [2]. This was suboptimal as the additional extra
attributes that only the 'level' prettifier had were exposed to other
prettifiers too.
To allow the additional attributes while keeping 'level' prettifier's
type separate, this patch uses an intersection Map type
CustomPrettifiers instead.
This is a breaking change.
[1] #551
[2] #550
This PR stacks on #551 to avoid conflicts down the line, so only the 2nd commit is actually relevant. (I'm not sure how to do cross-fork stacked PRs in GitHub...)
I guess the first commit in this PR will automatically disappear once #551 is (fast-forward?) merged. To review before that, we can use the link to the 2nd commit: 38cdaf27e1e2a1f33203aa249637e6e6d3086b78