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

How to stub dependencies with esm? #712

Closed
jzaefferer opened this issue Jan 22, 2019 · 16 comments · Fixed by #714
Closed

How to stub dependencies with esm? #712

jzaefferer opened this issue Jan 22, 2019 · 16 comments · Fixed by #714
Labels

Comments

@jzaefferer
Copy link

This is primarily a support request, but might turn into an enhancement for docs!

I'm trying to migrate from babel to esm, and I'm currently stuck trying to make some mocks work again.

What I'm starting with is some common setup code for mocha-based tests, like this:

var util = require('../src/lib/billing/util')
sinon.stub(util, 'getPricing').returns(Promise.resolve({})

As far as I can tell, that stub silently fails, since namespace objects are immutable.


Which I figured setting cjs.mutableNamespace to true would address, but I can't figure out how to do that. I did add this to my package.json:

  "esm": {
    "cjs": {
      "mutableNamespace": true
    }
  }

But then I get a new error that makes no sense to me:

SyntaxError: Missing export name 'diff' in ES module: [...]/node_modules/deep-diff/index.js

I suspect that I'm removing all cjs defaults by only setting the one option. But without examples for setting a single option and with no docs for the default values, I haven't been able to address that. I looked at

esm/src/package.js

Lines 64 to 97 in f15dfc7

const defaultOptions = {
await: false,
cache: true,
cjs: {
cache: false,
extensions: false,
interop: false,
mutableNamespace: false,
namedExports: false,
paths: false,
topLevelReturn: false,
vars: false
},
debug: false,
force: false,
mainFields: ["main"],
mode: "strict",
sourceMap: void 0,
wasm: false
}
const autoOptions = {
cjs: {
cache: true,
extensions: true,
interop: true,
mutableNamespace: true,
namedExports: true,
paths: true,
topLevelReturn: false,
vars: true
},
mode: "auto"
}
but the mode option makes this confusing, too - is mode: auto the default or not?


If a reduced test case has any value here, let me know and I'll put something together.

@jdalton
Copy link
Member

jdalton commented Jan 22, 2019

Hi @jzaefferer!

A reduced or small repro repo would help.

The mode is "auto" by default and cjs interop, including cjs.mutableNamespace is enabled by default (no need to opt-in) for all non-.mjs files. If you're trying to use .mjs you'll have a bad time because esm options don't extend to them (they're locked down because their support is experimental in Node).

@dnalborczyk

This comment has been minimized.

@jdalton

This comment has been minimized.

@dnalborczyk

This comment has been minimized.

@jdalton

This comment has been minimized.

@dnalborczyk

This comment has been minimized.

@dnalborczyk
Copy link
Contributor

dnalborczyk commented Jan 22, 2019

@jzaefferer just gave it a try, not sure if you tried to do something like this:

// index.js
import { stub } from "sinon"
import * as util from "./util.js"

stub(util, "getPricing").returns(Promise.resolve("$2"))

util.getPricing().then(val => console.log(val))
// util.js
export function getPricing() {
  return Promise.resolve("$1")
}
node -r esm index.js # output: $2

ps: make sure you're using the esm latest.

@jdalton
Copy link
Member

jdalton commented Jan 23, 2019

@jzaefferer Feel free to ping back with a repro and we will dig in.

In the meantime @dnalborczyk would you please open a PR to add a scenario test for Sinon so we ensure we don't regress this stub/spy case.

@jdalton jdalton closed this as completed Jan 23, 2019
@dnalborczyk
Copy link
Contributor

would you please open a PR to add a scenario test for Sinon so we ensure we don't regress this stub/spy case.

@jdalton yeah, sure, I'll have a look ...

@jzaefferer
Copy link
Author

Hello @jdalton! Thanks for looking into this in the first place. I've finally managed to put together a reduced test case: https://github.com/jzaefferer/esm-mock-repro

I suspect there's a silly mistake in there in the way I'm requiring/importing modules, though it did work fine pre-esm (with babel).

@jdalton
Copy link
Member

jdalton commented Jan 30, 2019

Hi @jzaefferer!

Thank you for the repro ❤️.

You've got a mix of things going on here. You don't need your common.js to be written in CJS. For example you could write it as:

common.js

import chai, { expect } from 'chai'
import sinon from "sinon"
import * as util from "../src/util"

global.chai = chai
global.expect = expect
global.sinon = sinon

sinon.stub(util, 'getPricing').returns(Promise.resolve({
  plans: ['a', 'b', 'c']
}))

then

const x = await util.getPricing()
expect(x).deep.equal({ plans: ['a', 'b', 'c'] })

passes.

The issue is that

var util = require('../src/util')

Is not the namespace object of

import * as util from "../src/util"

so if you stub the namespace object, the namespace object is stubbed. If you stub the exports then that's something else.

@dnalborczyk
Copy link
Contributor

@jzaefferer did you look at any of the examples above?

import sinon from "sinon";
import { expect } from "chai";
import * as util from "../src/util";

describe("lib", () => {
  it("tests stuff", async () => {
    sinon.stub(util, "getPricing").returns(Promise.resolve(["a", "b", "c"]));
    const x = await util.getPricing();
    expect(x).deep.equal(["a", "b", "c"]);

    util.getPricing.restore();
    const y = await util.getPricing();

    expect(y).deep.equal(["ABC", "FOO", "BAR"]);
  });
});

usually you would use either the cli options or the bridge (code), not both. you can also install sinon, chai etc. globally, if needed. I couldn't find a reference to common.js. is that a mocha thing?

@jzaefferer
Copy link
Author

Thanks, that makes sense. For some reason I missed that -r esm already loads esm, and accordingly the require = require('esm')(module) line isn't needed anymore, and the whole file can use imports.

@dnalborczyk I did! But it didn't (yet) help spot the issue with my common.js file. Also you're right that the lack of any reference to common.js is odd. I did mess that up. Originally I was using a mocha.opts file, but that seemed irrelevant here, but it was actually loading common/test as an extra module. I fixed that, and now its working very nicely: https://github.com/jzaefferer/esm-mock-repro/commit/d3c541f0a526cd5ad663f24489b99746d1704403 Thank you, too.

@jdalton
Copy link
Member

jdalton commented Feb 1, 2019

👋 @jzaefferer!

I've been thinking about this and your case will be common. With a couple lines of code I can make the namespace object be returned from require() when the mutable namespace option is enabled. This would have prevented a hiccup in the first place. I'll add that soon.

Update:

I added support for it. 😌

@jzaefferer
Copy link
Author

Nice! I'm glad this contributed a little bit to the project :-)

@jdalton
Copy link
Member

jdalton commented Feb 3, 2019

Patches 2d17fff, e1deed8;
Tests 562b82d, 15307b8, 54d0235, b576f38, f8d60c7;

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

Successfully merging a pull request may close this issue.

3 participants