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

Add valueStringifier for reporting #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pimartin
Copy link

This PR adds an optional valueStringifier property in the DetailedSpec that allows the user to customize the way the received value is displayed in the errors report.

This allows secrets to be redacted to prevent them from appearing in the logs for example.

If not provided, it defaults to using JSON.stringify, thus preserving backward compatibility.

Copy link
Owner

@lostfictions lostfictions left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution (and sorry to be slow to get around to a review). this seems like a reasonable feature to me.

it looks like the documentation you added is positioned incorrectly, and prettier needs to be run. could you also add a few tests for this? feel free to simply add them to the end of parse-env.test.ts.

what's the expected behaviour if valueStringifier throws?

Comment on lines +233 to +238
- `valueStringifier?: (value: string | TIn) => string`

A function that takes the received value (as a string) or the default value
(as `TIn`, the expected type after parsing) and returns the string that
should be displayed in the error report.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for adding documentation! looks like you inserted this in between the description of defaults and its example, so it should be moved downwards. could you also add the use-case of redacting values that you wrote up in the docstring?

@pimartin
Copy link
Author

pimartin commented Sep 4, 2023

Thanks for your comments, I will update the PR in accordance.

@lostfictions
Copy link
Owner

As a heads up, I've been thinking about this a bit this past week alongside how to fix #2 -- I may introduce a more low-level API for printing values that allows more fine-grained control over printing (including colorization), as well as a "compatibility" entrypoint whose default stringifier performs no colorization at all (to unblock usage on edge runtimes and in the browser). So things may shift around a bit -- feel free to hold off for now, I'll keep this use case in mind!

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.

2 participants