Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Command.setArgumentTransformer #1051

Closed
haakonnessjoen opened this issue Mar 5, 2021 · 8 comments · Fixed by #1106
Closed

Support for Command.setArgumentTransformer #1051

haakonnessjoen opened this issue Mar 5, 2021 · 8 comments · Fixed by #1106
Labels

Comments

@haakonnessjoen
Copy link

We are using this handy piece of code, to be able to work with hashes as objects in js:

function convertMapToArray(map: any): any[] {
	const result: any[] = []
	let pos = 0
	map.forEach(function (value: any, key: any) {
		result[pos] = key
		result[pos + 1] = value
		pos += 2
	})
	return result
}

function convertObjectToArray(obj: any): any[] {
	var result: any[] = []
	var keys = Object.keys(obj)

	for (var i = 0, l = keys.length; i < l; i++) {
		result.push(keys[i], obj[keys[i]])
	}
	return result
}

Redis.Command.setArgumentTransformer('hmset', function (args) {
	if (args.length === 2) {
		if (typeof Map !== 'undefined' && args[1] instanceof Map) {
			// utils is a internal module of ioredis
			return [args[0]].concat(convertMapToArray(args[1]))
		}
		if (typeof args[1] === 'object' && args[1] !== null) {
			return [args[0]].concat(convertObjectToArray(args[1]))
		}
	}
	return args
})

Redis.Command.setReplyTransformer('hgetall', function (result) {
	if (Array.isArray(result)) {
		const obj: any = {}
		for (var i = 0; i < result.length; i += 2) {
			obj[result[i]] = result[i + 1]
		}
		return obj
	}
	return result
})

But when using ioredis-mock, i get the following error:

TypeError: Cannot read property 'setArgumentTransformer' of undefined

      23 | }
      24 |
    > 25 | Redis.Command.setArgumentTransformer('hmset', function (args) {
         |               ^
      26 |      if (args.length === 2) {
      27 |              if (typeof Map !== 'undefined' && args[1] instanceof Map) {
      28 |                      // utils is a internal module of ioredis

      at Object.<anonymous> (src/lib/redis.ts:25:15)

So I guess that is not supported, as it works without ioredis-mock. It would be awesome to get support for that though.

@haakonnessjoen
Copy link
Author

Ok. So I see now that you are supposed to support it.
So the way I am using it now is with jest and this code:

const ioredisMock = require('ioredis-mock')
jest.setMock('ioredis', ioredisMock)

In the other module using ioredis, I tried console.log("Redis:", Redis) and console.log("Redis.Command:", Redis.Command), and got the following when it is being mocked:

Redis: [class RedisMock extends EventEmitter]
Redis.Command: undefined

And I am using ioredis-mock@5.2.4

@stipsan
Copy link
Owner

stipsan commented Mar 23, 2021

Yeah the thing is that it's tricky to make jest module mocking work when the mock is importing something from the original.

There's more alternative approaches in this thread: #568

It sounds like this approach is the most likely to succeed, but perhaps cost the most to implement, depending on how many import calls in your codebase that'll need to be updated: #568 (comment)

Perhaps the best solution here is if we added another import you could use, specifically for jest:

const ioredisMock = require('ioredis-mock/jest')
jest.setMock('ioredis', ioredisMock)

In the ioredis-mock/jest.js file we could simply inline import { Command } from 'ioredis'; to ensure that the jest module mocking system no longer have to deal with the circular dependency.

@stipsan
Copy link
Owner

stipsan commented Mar 23, 2021

@haakonnessjoen starting with v5.4 you can use this approach:

jest.mock('ioredis', () => require('ioredis-mock/jest'));

@stipsan stipsan closed this as completed Mar 23, 2021
@haakonnessjoen
Copy link
Author

I am still having the same issue:
TypeError: Cannot read property 'setArgumentTransformer' of undefined

Is it simply that ioredis-mock do not work with typescript at all, since ioredis is imported in the main code?

@stipsan stipsan reopened this Apr 20, 2021
@stipsan
Copy link
Owner

stipsan commented Apr 20, 2021

I am still having the same issue:

TypeError: Cannot read property 'setArgumentTransformer' of undefined

Is it simply that ioredis-mock do not work with typescript at all, since ioredis is imported in the main code?

Is that with the new ioredis-mock/jest approach? I'll go over the test suite, it should be testing this already 🤔

@silverwind
Copy link
Contributor

silverwind commented Dec 13, 2021

Yes, I see it with ioredis-mock/jest. My jest setupFiles has:

jest.mock("ioredis", () => require("ioredis-mock/jest"));

My application uses:

Redis.Command.setReplyTransformer("hgetall", result => {
  const arr = [];
  for (let i = 0; i < result.length; i += 2) {
    arr.push([String(result[i]), String(result[i + 1])]);
  }
  return arr;
});

During the jest run, it throws:

TypeError: Cannot read properties of undefined (reading 'setReplyTransformer')

@silverwind
Copy link
Contributor

silverwind commented Dec 13, 2021

I think the issue is that setReplyTransformer and setArgumentTransformer are implemented as RedisMock.prototype.Command, but they should not be on the prototype, but on RedisMock.Command itself as per docs.

> require("ioredis").Command.setReplyTransformer
[Function: setReplyTransformer]
> require("ioredis-mock/jest").Command.setReplyTransformer
Uncaught TypeError: Cannot read properties of undefined (reading 'setReplyTransformer')

silverwind added a commit to silverwind/ioredis-mock that referenced this issue Dec 13, 2021
The transformers implementation was never compatible with ioredis, this
fixes it and also adds much-needed tests.

Fixes: stipsan#1051
silverwind added a commit to silverwind/ioredis-mock that referenced this issue Dec 13, 2021
The transformers implementation was never compatible with ioredis, this
fixes it and also adds much-needed tests.

Fixes: stipsan#1051
silverwind added a commit to silverwind/ioredis-mock that referenced this issue Dec 13, 2021
The transformers implementation was never compatible with ioredis, this
fixes it and also adds much-needed tests.

Fixes: stipsan#1051
silverwind added a commit to silverwind/ioredis-mock that referenced this issue Jan 21, 2022
The transformers implementation was never compatible with ioredis, this
fixes it and also adds much-needed tests. Also, this includes a fix for
hgetall with transformers.

Fixes: stipsan#1051
stipsan added a commit that referenced this issue Jan 21, 2022
* fix: Fix transformers

The transformers implementation was never compatible with ioredis, this
fixes it and also adds much-needed tests. Also, this includes a fix for
hgetall with transformers.

Fixes: #1051

* Update test/command.js

Co-authored-by: Cody Olsen <stipsan@gmail.com>

Co-authored-by: Cody Olsen <stipsan@gmail.com>
@stipsan
Copy link
Owner

stipsan commented Jan 21, 2022

🎉 This issue has been resolved in version 5.8.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants