Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Unable to mock .mjs files #786

Open
theKashey opened this issue Apr 24, 2019 · 10 comments
Open

Unable to mock .mjs files #786

theKashey opened this issue Apr 24, 2019 · 10 comments
Labels

Comments

@theKashey
Copy link

theKashey commented Apr 24, 2019

Let's define two files - foo and index. Then lets mock them using Module._load based mocker.

// foo.js
export default () => 'FOO';
// index.js
import foo from './foo';
export default () => foo();

Scenario 1 - all .js

  • mock foo.js to return bar
  • require foo.js - 👍
  • require index.js - 👍

Scenario 2 - foo.mjs + index.js

  • mock foo.mjs to return bar
  • require foo.mjs - 👍
  • require index.js - 👍

Scenario 3 - foo.mjs + index.mjs

  • mock foo.mjs to return bar
  • require foo.mjs - 👍
  • require index.mjs - 👎 <<---------, while foo.mjs is still mocked.

Scenario 3 - foo.js + index.mjs

Cannot load module (foo.js) from .mjs


Probably this is a feature, not a bug. But how then mock .mjs?

Example: https://github.com/theKashey/rewiremock-esm-repro

@jdalton
Copy link
Member

jdalton commented Apr 24, 2019

Hi @theKashey!

Can you clarify things a bit more. It's not clear to me what is and isn't working based on your example repo and the scenario's you've posted. I don't know what "require foo.mjs" or "require index.js" means in this scenario.

@theKashey
Copy link
Author

theKashey commented Apr 24, 2019

spec.js provides a simple config

rewiremock('./foo').withDefault(() => 'fake-foo'); // mock `foo`
index = (await import('./index')).default; // require `index`

Then I am trying to run different scenarios manually renaming files.

  • everything works when they are js
  • mocking + require of foo.mjs is working. Ie still goes thought Module._load
  • mocking + requiring foo.mjs via index.js is working.
  • mocking + requiring foo.mjs via index.mjs is not working. Look like in case of mjs Module._load is working only for a first file. Later it uses something another.

@jdalton
Copy link
Member

jdalton commented Apr 24, 2019

Thank you!

Ideally mocking shouldn't work for .mjs at all. What you've uncovered is gaps in our limitation logic to prevent it. So my fixes here would just be making all of the existing places where mocking .mjs works prevented.

@theKashey
Copy link
Author

🤯 So - the old ways to mock are dying, but what about new ways?
Loaders hooks and loaders maps could handle it, but not in a test-friendly way.

As long as (module-level) dependency mocking is an essential part of any testing - it shall be possible. We don't have any other variant (sinon stubbing is not a variant).

@jdalton
Copy link
Member

jdalton commented Apr 24, 2019

The .mjs file extension has always lacked esm option support. So these thing that you thought were possible were really never intended for .mjs. Stubbing/spys and mocking work fully with .js.

@theKashey
Copy link
Author

Yep. That's why I am asking for advice. Right now it makes .mjs impossible to use. I am quite addicted to dependency mocking, and think that it's a good pattern, and without proper support would have to migrate to more classical DI, which would make everything more classish and complex (as seen in Java).

There should be a way to mock mjs.

@jdalton
Copy link
Member

jdalton commented Apr 24, 2019

When Node officially supports mocking then you can I guess. For esm you can always use .js. If you insist on .mjs you're likely going to have a bad time across multiple tools beyond esm though.

@theKashey
Copy link
Author

Ok, so I have to open issue for a node/modules until it's not too late.

@theKashey
Copy link
Author

The problem is tracked elsewhere - nodejs/modules#98

@jdalton jdalton reopened this Apr 24, 2019
@jdalton
Copy link
Member

jdalton commented Apr 24, 2019

Thanks @theKashey!

But there's still a consistency issue on our end to track 😸

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants