Replace console.warn with logger.warn: componentParser.js

[nbriannl]

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [ ] Documentation update
• [ ] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [X] Other, please explain: Finishing up a todo from #1057

What is the rationale for this request?
This is a follow up to #1057 where console.warn is used instead of logger.warn due to a bug as raised in #1060. The test no long fail in #1117 by mocking logger in the parser.test.js. Although the approach could be improved.

What changes did you make? (Give an overview)
Replaced all console.warn with logger.warn as indicated by the TODO signposts.

Testing instructions:
Run tests to check if anything regressed

markbind serve the example code in the gist.
https://gist.github.com/nbriannl/5c44b3ee8a8af78c45a7a4f3659ce196

You should now see

Proposed commit message: (wrap lines at 72 characters)

Replace console.warn with logger.warn,

A follow up to #1057.
Let us also remove console.error as the error handler
And directly set the error handler to be logger during init

[ang-zeyu]

should we fix the root cause of the tests failing first instead? ( mocking fs for winston )

[nbriannl]

should we fix the root cause of the tests failing first instead? ( mocking fs for winston )

Even if we fix the root cause, we would still have to do this change. Therefore, I thought of doing it now since the bug is fixed, albeit not as ideally. 😄

Should I raise a new issue or rewrite the #1060 ?

[ang-zeyu]

Ahh ok, I'm fine with either. If that's the case perhaps could replace the usages in parser.js as well?

*edited your comment when quoting by accident

[nbriannl]

Ahh ok, I'm fine with either. If that's the case perhaps could replace the usages in parser.js as well?

*edited your comment when quoting by accident

Sounds good, I can do that here.

[nbriannl]

Should i be changing this line as well?

markbind/src/lib/markbind/src/parser.js

Line 37 in 68eaefb

this._onError = this._options.errorHandler || console.error;

[ang-zeyu]

Should i be changing this line as well?

Yes, think we could remove it completely though, and all instances of errorHandler being passed as an option into the parser constructor. It was likely a workaround for the root problem

[nbriannl]

Should i be changing this line as well?

Yes, think we could remove it completely though, and all instances of errorHandler being passed as an option into the parser constructor. It was likely a workaround for the root problem

Hm, not entirely sure what you mean. _onError is also used in part relating to componentPreprocessor.preProcessComponent()

[ang-zeyu]

Hm, not entirely sure what you mean. _onError is also used in part relating to componentPreprocessor.preProcessComponent()

It's a workaround for not being to import logger directly in parser

• when we the unit test is run, _onError defaults to console.error
• when running anything else from a user standpoint ( e.g. markbind build ), errorHandler is set to logger.error. Should be unncessary with the changes here now

[ang-zeyu]

LGTM 👍