Replace dependecy Logger4 with Winston

[rofrantz]

Hi,

Your library works perfect for backend and also for frontend apps, however after upgrading from Angular 11 to Angular 12 (which underneath upgraded from Webpack 4 to 5) the application doesn't compile anymore because of the usage of "fs" and "path" which obviously are not supported by browsers.

Initially I wanted to do myself a PR with the upgrade (and I'm still willing to help) but once I've cloned your repo the 1st thing that hit was the setup with the missing sensitive/sensitive.json file for obvious security reasons; I've fixed that no pb. However I can't test it, because in your package.json you have under scripts section:

"test": "echo \"Error: no test specified\" && exit 1"

And this is where I'm blocked currently, since I've only found a way to launch mocha for the sandbox/sandbox.ts file only. Do you think that it would be possible to fix that in your repo ? Also, out of curiosity why do you chai as a dependency ? are you using mocha or chai since I don't see any usage of it ?

This is Winston.

[peterszombati]

Hi,

currently I don't have time for develop this because I don't getting any money from that, it's just my hobby project, feel free for pull request. it should compile I will check that but with React I could use it. also with logger4 you can bind winston or any logger what you want

[rofrantz]

That sounds great however keep in my mind that you haven't answered my question: how do I ran tests to make sure everything's still ok since your package.json test script is invalid

[peterszombati]

you can create your tests, I didnt create because I didnt have time for that. I just used mocha for some test when I created something it was easier to run & test it works or not.

[rofrantz]

But you do have here and in the build package you also include sandbox unfortunately which are injecting those "fs" and "path" unwanted dependencies for a frontend compatible library

[peterszombati]

Ah okay, I will move that from here

[peterszombati]

@rofrantz I moved away test files from src directory, can you check is it working or not

[rofrantz]

I've noticed however they haven't been moved completely as you can see here:

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "path": require.resolve("path-browserify") }'
        - install 'path-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "path": false }

./node_modules/xapi-node/build/modules/parseLogin.js:4:11-24 - Error: Module not found: Error: Can't resolve 'fs' in '\node_modules\xapi-node\build\modules'

Beside that, Logger4 (I've only noticed now that it's also yours 😋) is still a direct dependency to xapi-node which 'causes further errors as reported initially when trying to compile:

./node_modules/logger4/build/core/Logger4.js:4:11-24 - Error: Module not found: Error: Can't resolve 'fs' in '\node_modules\logger4\build\core'

./node_modules/logger4/build/core/Logger4.js:5:13-28 - Error: Module not found: Error: Can't resolve 'path' in '\node_modules\logger4\build\core'

[peterszombati]

@rofrantz I fixed both logger4 and xapi-node, now you can check it

[peterszombati]

@rofrantz later I can improve better logging for xapi-node

[rofrantz]

@peterszombati this issue starts reoccurring starting with version 2.5.7 since you're using Logger4 which relies on NodeJS making this library unusable on frontend in the browsers as mentioned before:

./node_modules/node-gyp-build/index.js:3:9-22 - Error: Module not found: Error: Can't resolve 'os' in 'PROJECT_PATH\node_modules\node-gyp-build'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "os": require.resolve("os-browserify/browser") }'
        - install 'os-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "os": false }

So required deps injected by Logger 4 are:

• os
• zlib
• stream
• net
• tls
• crypto
• http
• https

[peterszombati]

logger4 dependency removed 2.8.0