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

Remove defaults #1

Open
marchaos opened this issue Mar 6, 2024 · 3 comments
Open

Remove defaults #1

marchaos opened this issue Mar 6, 2024 · 3 comments

Comments

@marchaos
Copy link

marchaos commented Mar 6, 2024

Hey. In https://github.com/thelicato/pino-rotating-file-stream/blob/main/src/index.ts you set some default values, but in my case I don't want to have an interval, but with those I cannot set it to undefined.

Are you able to remove those default values so that those can be undefined?

@thelicato
Copy link
Owner

Hello, the interface PinoRotatingFileStreamOptions extends the Options interface defined in the rotating-file-stream package.
It should be defined here: https://github.com/iccicci/rotating-file-stream/blob/master/index.ts#L55

Since the values you are referring to are optional I think you can manually set them to undefined/none.
I haven't tested it out, so please let me know if it works.

@longzheng
Copy link

longzheng commented Oct 3, 2024

I'm also running into this problem with compress: false which doesn't work due to the defaults. (Sorry compress is a bad example since false isn't supported by rotatingfile-stream`)

Hello, the interface PinoRotatingFileStreamOptions extends the Options interface defined in the rotating-file-stream package. It should be defined here: https://github.com/iccicci/rotating-file-stream/blob/master/index.ts#L55

Since the values you are referring to are optional I think you can manually set them to undefined/none. I haven't tested it out, so please let me know if it works.

This doesn't work because the defaults are using || which means a falsy value of undefined or false will all fallback to the specified defaults

size: size || '100M',
maxSize: maxSize || '1G',
interval: interval || '7d',
compress: compress || 'gzip',

For example

// assume interval = undefined

undefined || '7d'
// '7d'

@longzheng
Copy link

Sorry actually looking at the rotating-file-stream library, it looks like they actually don't accept undefined for a lot of values, including interval

Error: Don't know how to handle 'options.interval' type: undefined
    at Object.interval (C:\Users\longz\Documents\GitHub\pino-rotating-file-stream\node_modules\rotating-file-stream\dist\cjs\index.js:438:19)
    at checkOpts (C:\Users\longz\Documents\GitHub\pino-rotating-file-stream\node_modules\rotating-file-stream\dist\cjs\index.js:535:20)
    at createStream (C:\Users\longz\Documents\GitHub\pino-rotating-file-stream\node_modules\rotating-file-stream\dist\cjs\index.js:579:18)

I'm not sure why they made the type optional

https://github.com/iccicci/rotating-file-stream/blob/e4a46274e672920fcb38b0cdf65ddc3e2ea7da6c/index.ts#L62

but then don't actually support undefined

https://github.com/iccicci/rotating-file-stream/blob/e4a46274e672920fcb38b0cdf65ddc3e2ea7da6c/index.ts#L601

With that in mind then, I guess this library always setting some defaults even for falsy values makes sense.

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

No branches or pull requests

3 participants