perf(client): Replace chalk with kleur/color
#18900
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signgicantly reduces bundle size (from 280K to 255K) and improves `require(runtime)` performance by 5-10ms.
SevInf
commented
Apr 24, 2023
CodSpeed Performance ReportMerging #18900 Summary
|
SevInf
commented
Apr 24, 2023
aqrln
reviewed
Apr 24, 2023
aqrln
reviewed
Apr 24, 2023
packages/client/src/runtime/utils/createErrorMessageWithContext.ts
Outdated
Show resolved
Hide resolved
aqrln
reviewed
Apr 24, 2023
aqrln
reviewed
Apr 24, 2023
…t.ts Co-authored-by: Alexey Orlenko <alex@aqrln.net>
Jolg42
reviewed
Apr 25, 2023
Jolg42
reviewed
Apr 25, 2023
Jolg42
approved these changes
Apr 25, 2023
aqrln
reviewed
Apr 25, 2023
|
|
||
| export class DbCommand implements Command { | ||
| public static new(cmds: Commands): DbCommand { | ||
| return new DbCommand(cmds) | ||
| } | ||
|
|
||
| private static help = format(` | ||
| ${process.platform === 'win32' ? '' : chalk.bold('🏋️ ')}Manage your database schema and lifecycle during development. | ||
| ${process.platform === 'win32' ? '' : bold('🏋️ ')}Manage your database schema and lifecycle during development. |
There was a problem hiding this comment.
bold with emoji probably doesn't have any visible effect
There was a problem hiding this comment.
although not sure, maybe it might depending on the font
There was a problem hiding this comment.
there's quite a lot of these bold emojis as far as I see, maybe there was some intention behind it
There was a problem hiding this comment.
my guess would be, they used to be some plain unicode symbol for which bold made sense and got changed to emoji later
aqrln
reviewed
Apr 25, 2023
| import terminalLink from 'terminal-link' | ||
|
|
||
| export function link(url): string { | ||
| return terminalLink(url, url, { | ||
| fallback: (url) => chalk.underline(url), | ||
| fallback: (url) => underline(url), |
There was a problem hiding this comment.
nit:
Suggested change
| fallback: (url) => underline(url), | |
| fallback: underline, |
(feel free to ignore though)
aqrln
approved these changes
Apr 25, 2023
SevInf
added a commit
that referenced
this pull request
May 3, 2023
Address some of the feedback added to #18900 after it got merged
SevInf
added a commit
that referenced
this pull request
May 3, 2023
Address some of the feedback added to #18900 after it got merged
SevInf
added a commit
that referenced
this pull request
May 3, 2023
Address some of the feedback added to #18900 after it got merged
SevInf
added a commit
that referenced
this pull request
May 3, 2023
Address some of the feedback added to #18900 after it got merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signgicantly reduces bundle size (kleur is <1KB in size, which reduces library runtime size from 280K to 255K) and
improves
require(runtime)performance by 5-10ms:Few implementation notes:
kleuris limited to 16 colors, in particular,*Brightversions of the colors are not supported. I replaced them with non-bright versions, which still look ok in the several places I've checked, and IMHO, is a fair price to pay for size/startup performance improvements. If you disagree - let me know.migrateandclitechnically did not need to migrate, but since we moving and sharing code between different packages from time to time, I've decided it would be better if we handle colors the same way everywhere and went for completechalkremoval.Close prisma/client-planning#341