Skip to content
This repository was archived by the owner on Jan 25, 2020. It is now read-only.

Using the modules's name instead of modules's name.property.#7

Open
johnkchiu wants to merge 1 commit into
krakenjs:masterfrom
johnkchiu:MultiFileLogger
Open

Using the modules's name instead of modules's name.property.#7
johnkchiu wants to merge 1 commit into
krakenjs:masterfrom
johnkchiu:MultiFileLogger

Conversation

@johnkchiu
Copy link
Copy Markdown

Currently for config such as:

modules: {
        mongodb: {
            name: 'winston-mongodb',
            property: 'MongoDB'
        }
    },

MongoDB is used as the transport name. This shouldn't be the case because:

  1. property is an optional value so if not provided, it'll be undefined.
  2. This limits you from using the same module for multiple transports.

(This also allows me to use winston-file transport twice for issue #6.)

@totherik
Copy link
Copy Markdown
Contributor

Thanks @johnkchiu. I think this is going to be safe, albeit not backward compatible so will require a major version change. There's a caveat I remember dealing with, however can't remember the details, but it has to do with naming. Internally, winston capitalizes the exported transport constructors, but internally IIRC the instances are mapped to lowercase names. So uniqueness is, unfortunately, case-insensitive which may cause subtle bugs.

Other than ensuring the internal transports can be extended correctly, can you please add tests for issue #6 if this is intended to fix it. (This will be a major rev so I'd like to have tests to back the anticipated behavior.) Thanks again.

@johnkchiu
Copy link
Copy Markdown
Author

I'm not a fan of this change either, but having config for the same transport is useful. Another approach is to add some addition properties to transport or module config:

Adding module to Transport ?

    return pine(name, {
        transports: {
            file: {
                level: 'debug',
                filename: 'server.log'
            },
            // Error file logging
            fileError: {
                level: 'error',
                filename: 'error.log'
                module: 'File' 
            }
        }
    });

Adding $ref to Module ?

    return pine(name, {
        transports: {
            file: {
                name: 'file.debug',
                level: 'debug',
                filename: 'server.log'
            },
            // Error file logging
            fileError: {
                name: 'file.error',
                level: 'error',
                filename: 'error.log'
            }
        },
        modules: {
            fileError: {
                '$ref': 'File'
            }
        }

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants