effect icon indicating copy to clipboard operation
effect copied to clipboard

`Logger.replace` in cjs yarn monorepo doesn't work if called from another package

Open sukovanej opened this issue 1 year ago • 4 comments

What version of Effect is running?

2.0.0-next.50

What steps can reproduce the bug?

When testing a cjs monorepo package using vitest, e.g. Logger.replace doesn't have any effect when it's being called from a different package than the one being tested.

Repro: https://github.com/sukovanej/effect-cjs-yarn-monorepo-repro/

What is the expected behavior?

combinators manipulating fiber refs can be defined in different cjs packages and used with vitest

What do you see instead?

No response

Additional information

No response

sukovanej avatar Oct 23 '23 15:10 sukovanej

cc @andarist - This is caused by dual-package hazard, even though effect is built with preconstruct (at the moment). What happens here is that we forgot to define loggers as global so two instance of defaultLogger are created and replace doesn't work, we can fix it by using a global for loggers but it's strange because this should not occur in the first place. Maybe vitest prefers the pure modules and ignores the esm proxy modules?

mikearnaldi avatar Oct 23 '23 19:10 mikearnaldi

Vitest is using some weird strategy of bundling/resolving some things on its own but yet leaving some other things for node to resolve. I didn't dig deep enough to know what exact heuristic they are using to do this (they use workers so call stacks are not that useful) but I'm pretty sure of that conclusion. I couldn't observe effect's resolution from pkg-1 to go through Vite's resolveId function (which indicates that they didn't resolve it and left it to node). At the same time, I was able to "force" it to go through that after adding package.json#exports.{module,default} (and appropriate ESM file) to pkg-1/package.json.

I think this should be reported to the Vite team regardless of how you decide to resolve it here. From my PoV, this is clearly an issue on their side.

Andarist avatar Oct 23 '23 22:10 Andarist

This is solved on our side, keeping it open as a reference to track vitest

mikearnaldi avatar Oct 24 '23 21:10 mikearnaldi

Confirming the problem resolved on my end after the effect update. Thanks!

sukovanej avatar Oct 25 '23 22:10 sukovanej

Upstream vitest closed the issue - I don't think there is any further action needed here?

tim-smart avatar Apr 02 '24 23:04 tim-smart