uuidjs / uuid

Generate RFC-compliant UUIDs in JavaScript
MIT License
14.61k stars 902 forks source link

v7 methods cannot be stubbed #377

Closed CareLuLu-Gabriel closed 4 years ago

CareLuLu-Gabriel commented 4 years ago

Our unit tests that use uuid stopped working. It seems that sandbox.stub(uuid, 'v4') no longer works.

const uuid = require('uuid');
const sinon = require('sinon');

const sandbox = sinon.createSandbox();
sandbox.stub(uuid, 'v4').returns('uuid');

console.log(uuid.v4());
// prints some uuidv4 instead of 'uuid'

It seems to be related to the way defineProperty is being used. Not sure if there are other implications of replacing the defineProperty by simply setting the property, but switching this:

"use strict";

Object.defineProperty(exports, "v4", {
  enumerable: true,
  get: function () {
    return _v3.default;
  }
});

var _v3 = _interopRequireDefault(require("./v4.js"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

to the following works.

"use strict";

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var _v3 = _interopRequireDefault(require("./v4.js"));

exports.v4 = _v3.default;
EJMason commented 4 years ago

I'm currently running into the exact same issue. My guess is that it has to do with there not being any default export.

I tried this and it also did not work:

import { v4 } from 'uuid'

sandbox.stub(v4).returns('test-id')
TrySound commented 4 years ago

ES Modules will not allow you to patch module object (import * as uuid from 'uuid') either. You need to find another way to replace implementation like mocking modules in jest or dependency injection.

CareLuLu-Gabriel commented 4 years ago

I'm using this work around. Created a wrapper lib for uuid.

// lib/uuid.js

const uuid = require('uuid');

function v4() {
  return uuid.v4();
}

exports.v4 = v4;
broofa commented 4 years ago

@EJMason While it's true there's no default export for the ES6 module version, I don't think that's the issue. Rather, we've switched to using babel/rollup for compiling CommonJS, ESM, and UMD-specific files. One side-effect of this is that babel now builds the CommonJS v4 export as follows:

Object.defineProperty(exports, "v4", {
  enumerable: true,
  get: function () {
    return v4.default;
  }
});

I.e. The v4 property is now enumerable, but is not configurable or (more importantly) writable, so cannot be replaced. I'll let @ctavan chime in on this, but I believe the idea is that preventing modification by malicious / clumsy 3rd party code is generally considered to be a good thing.

broofa commented 4 years ago

@CareLuLu-Gabriel Per my previous comment, I suspect providing a exports with writable properties will solve this problem. I.e. A module that consists simply of ...

Object.assign(exports, require('uuid'));
CareLuLu-Gabriel commented 4 years ago

@broofa yes, that's what I did in a way less elegant way :)

I can see preventing modification as a good thing (specially in the browser). The downside is stubbing. What about using the main and browser entry points on package.json to allow stubbing on node?

ctavan commented 4 years ago

As @broofa already explained we have migrated this library to use ESModules and per the spec ESModule exports cannot be dynamically modified, so in fact everything behaves as expected. Obviously this new behavior differs from the way things behaved with CommonJS exports with the downside of not being as easily mockable as before.

I found these discussions provided some helpful context:

As suggested above, if you really want to continue to mock the uuid module you are left with building a wrapper or using something like https://github.com/speedskater/babel-plugin-rewire#readme.

If all you want to achieve is a static output you could also try to mock the global crypto.randomBytes() method from the node crypto library or crypto.getRandomValues() for the browser.

Or you could provide your own random number generator as described in https://github.com/uuidjs/uuid#version-4-random and done in this test.

@CareLuLu-Gabriel with respect to your specific question: Looking forward I don't think that modules should continue to be mockable in the way they were in CommonJS times so I'm reluctant on going one step back here. I assume that ESModules will become more and more common in Node.js-land pretty soon so we'll see these mockability issues arise for many other modules as well. I think we'll need other solutions than just sticking with CommonJS behavior.

Meemaw commented 4 years ago

You can do this:

jest.mock('uuid', () => {
  return {
    v4: () => '1',
  };
});

Not ideal if you want to mock using sandboxes, but it works.

azizur commented 4 years ago

I have found this works really nice.

At the top of test file:

// your regular imports here

jest.mock('uuid', () => ({ v4: () => '00000000-0000-0000-0000-000000000000' }));

// describe('test suite here', () => {

in the actual code:

import { v4 as uuidv4 } from 'uuid';
bompi88 commented 4 years ago

This seems to be working for me for version 8.3.1. Still a hack though, but I don't want to rely on proxyquire etc.

const uuidV4 = require('uuid/dist/v4');
const sinon = require('sinon');

const sandbox = sinon.createSandbox();
sandbox.stub(uuidV4, 'default').returns('uuid');
shackpank commented 4 years ago

@bompi88 that will stop working in more recent stable versions of node - it works on node 10 but node 12 and above enforces that you can only require things the package intended to export, and will throw an ERR_PACKAGE_PATH_NOT_EXPORTED error

rooby commented 3 years ago

This seems to work for me with jasmine:

import * as uuid from 'uuid';
...
spyOnProperty(uuid, 'v4').and.returnValue(() => 'f3b3a923-61b3-4d6f-9a7e-632061825358');