Circular reference warnings in the main library export #1886
Comments
|
Confirmed with node v14.17.6 |
|
Confirmed with node v20.10.0 |
|
If anyone wants to help figure out where this is coming from, PR would be gladly welcomed (maintainers are stretched thin). I'm happy to review any potential fix though. |
|
Took a look at things here this morning and the circular reference causing the problem is actually in It should probably be fixed there, although it can be worked around here by requiring Also worth noting: this issue is also currently breaking the // Broken:
const winston = require('winston');
new winston.Transport.LegacyTransportStream({ transport: { log(info, cb) { cb(); }, on() {} }});
// Logs:
// new winston.Transport.LegacyTransportStream({ transport: { log(info, cb) { cb(); }, on() {} }});
// ^
//
// TypeError: winston.Transport.LegacyTransportStream is not a constructor
// at Object.<anonymous> (/Users/rbrackett/Dev/winston/testme.js:2:1)
// at Module._compile (node:internal/modules/cjs/loader:1241:14)
// at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
// at Module.load (node:internal/modules/cjs/loader:1091:32)
// at Module._load (node:internal/modules/cjs/loader:938:12)
// at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
// at node:internal/main/run_main_module:23:47
// But this works fine when referencing legacy transport directly:
const LegacyTransportStream = require('winston-transport/legacy');
new LegacyTransportStream({ transport: { log(info, cb) { cb(); }, on() {} }}); |
There is a circular dependency between the `TransportStream` (formerly in `index.js`) and `LegacyTransportStream` (in `legacy.js`). This causes some funky problems when you import the legacy module and later import the main module, as seen in winstonjs/winston#1886. I've resolved the circular issue here by definiting the main implementation of `TransportStream` in a separate module (which `LegacyTransportStream` now depends on) and then doing the work of decorating it with a `.LegacyTransportStream` property in `index.js` (and that's pretty much all the index module does).
|
Filed a PR to fix it upstream in Once that’s merged and released, then this package will need to update its dependency on |
* Fix circular dependency between transport streams There is a circular dependency between the `TransportStream` (formerly in `index.js`) and `LegacyTransportStream` (in `legacy.js`). This causes some funky problems when you import the legacy module and later import the main module, as seen in winstonjs/winston#1886. I've resolved the circular issue here by definiting the main implementation of `TransportStream` in a separate module (which `LegacyTransportStream` now depends on) and then doing the work of decorating it with a `.LegacyTransportStream` property in `index.js` (and that's pretty much all the index module does). * Resolve lint warning while I'm here
|
Thanks @Mr0grog for the help on this one! I merged the |
|
No problem! Do I need to do a PR to update the minimum version of winston-transport in |
|
Dependabot should automatically open a PR tomorrow but if you want to beat the bot to it, go for it :) |
|
Oh, if it's configured to update the minimums, then I'll just leave it to Dependabot. Nothing more to do here. 👍 |
Please tell us about your environment:
winstonversion?winston@2winston@3node -voutputs:v15.8.0Linux $HOSTNAME 5.8.0-41-generic #46~20.04.1-Ubuntu SMP Mon Jan 18 17:52:23 UTC 2021 x86_64 x86_64 x86_64 GNU/LinuxWhat is the problem?
starting up an interactive shell prints lots of warnings after requiring the winston library, and then simply typing out its name to see its exports
Example output
What do you expect to happen instead?
I expect the output to not contain warnings, like so:
instead this is the output:
The text was updated successfully, but these errors were encountered: