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

Update winston to 3.x #11

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

Update winston to 3.x #11

wants to merge 3 commits into from

Conversation

Terseus
Copy link

@Terseus Terseus commented Apr 7, 2021

This change is needed for foreversd/forever#1077.

Note that winston 3 requires Node.js >= 6.4.0 so this change breaks compatibility with older Node.js versions.

@Terseus
Copy link
Author

Terseus commented Apr 24, 2021

@zoobot I cannot reproduce the issue anymore in this branch.

Anyway I've added a test to prevent other warnings in the future.

You can test the behavior with this script:

var cliff = require('./lib/cliff.js');

process.on('warning', function() {
  console.error("Oops, warning detected!");
});

var rows = [
    ['a', 'b'],
];


cliff.putRows('info', rows, ['red', 'blue']);

When I run it on master branch:

$ node --version
v15.12.0
$ node ./warning-test.js
info:    a b
(node:39912) Warning: Accessing non-existent property 'padLevels' of module exports inside circular dependency
(Use `node --trace-warnings ...` to show where the warning was created)
Oops, warning detected!

When I run it on this branch (be sure to delete node_modules and execute npm install to update winston):

$ node --version
v15.12.0
$ node ./warning-test.js
info:    a b

If you're still having the issue, please provide an small example so I'm able to test it.

Copy link

@meissnere meissnere left a comment

Choose a reason for hiding this comment

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

wonderful. let's pull this into flatiron master to fix these winston issues

Copy link

@meissnere meissnere left a comment

Choose a reason for hiding this comment

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

wonderful. let's pull this into flatiron master to fix these winston issues

lib/cliff.js Outdated
});

//
// Set the default logger for cliff.
//
cliff.logger = new winston.Logger({
transports: [new winston.transports.Console()]
cliff.logger = new winston.createLogger({

Choose a reason for hiding this comment

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

Suggested change
cliff.logger = new winston.createLogger({
cliff.logger = winston.createLogger({

Copy link
Author

Choose a reason for hiding this comment

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

Oops you're right, I'll add it in a87d5da, thanks!

This warning came from winston 0.8.x:

    Warning: Accessing non-existent property 'padLevels' of module
    exports inside circular dependency

See foreversd/forever#1077
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.

3 participants