standard-things / esm

Tomorrow's ECMAScript modules today!
Other
5.26k stars 146 forks source link

esm loads two versions of the same node module with jest #734

Closed thomas-jeepe closed 5 years ago

thomas-jeepe commented 5 years ago

Best explanation is a reproduction right here :)

Follow the instructions in the readme.

The test should pass, as the state of the util library should remain the same between tests and other-util, but it looks like esm loads it twice.

My system information:

yarn -v
1.10.1
node -v
v11.7.0
npm -v
6.0.0

Windows 10 and ran commands through Powershell.

jdalton commented 5 years ago

Hi @thomas-jeepe!

It's related to the change for #492. \cc @dnalborczyk

Update:

I reverted 25e9c87 at https://github.com/standard-things/esm/commit/b876ad0c81570c1fdd35c8138e27ea1d6f9ef053.

@dnalborczyk I think the appropriate action for your situation is to clear the cache in your test:

import Module from "module"

Module._cache = {}
dnalborczyk commented 5 years ago

@jdalton I'm not a jest expert, but cjs modules are behaving the same, hence I added those to the tests for comparison: https://github.com/dnalborczyk/esm-jest-repro/tree/jest-scope/tests

jdalton commented 5 years ago

That's because jest is going through its own managed cache. When you go through esm you're deferring to esm's managed cache. So you'll need to modify it as shown above to get the behavior you're wanting.

Update:

If you spot where jest is storing its module data (maybe on the jest object) I might be able to hook into using that by default.

dnalborczyk commented 5 years ago

I'm not really sure. if you think that's the correct behavior. I suppose it depends what you expect esm to do for users using jest. if people are aware of this I guess it's just fine as well...?

dnalborczyk commented 5 years ago

I'm would assume that jest with babel (transpiling to cjs as well) behave the same and I'm guessing that once esm fully supports jest users would expect the same behavior?

but if jest is taking care of removing the cache right now and it doesn't know about esm, then letting the user take care of it might be the right thing to do?

it's really your call ...

dnalborczyk commented 5 years ago

one thing tho I don't understand:

if I use the same bridge in both test files, shouldn't jest hook into the mentioned cache as well, and therefore remove it?

// bridge.js
require = require('esm')(module)
module.exports = require("./es.js")
// es.js
let scopeCounter = 0

export function getScopeCounter() {
  return ++scopeCounter
}
// test-1.js
const { getScopeCounter } = require('../bridge.js')

test('test', () => {
  expect(getScopeCounter()).toBe(1)
})
// test-2.js
const { getScopeCounter } = require('../bridge.js')

test('test', () => {
  expect(getScopeCounter()).toBe(1)
})
jdalton commented 5 years ago

@dnalborczyk

Never mind, I fixed it for both your and @thomas-jeepe's scenarios with a simple one-line change.

Patches https://github.com/standard-things/esm/commit/daa6ca6bbf03e4004f4f6680a0d929d927838642, https://github.com/standard-things/esm/commit/d372c498f67245f25a52f370a71abea5fdb4ad0c;

dnalborczyk commented 5 years ago

@jdalton mmmhhh.. I thought the scenarios where actually the same, but looking at it closer again I just noticed that they weren't. @thomas-jeepe is using 1 test file. in that case I'm assuming jest would behave as expected with cjs as well and should instantiate the module only once.

jdalton commented 5 years ago

esm v3.2.9 is released 🎉

thomas-jeepe commented 5 years ago

esm v3.2.9 works perfectly with my real project now! Thanks!