-
Notifications
You must be signed in to change notification settings - Fork 419
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
fix: modified imports to work when esModuleInterop is disabled #132
Changes from 1 commit
c7462ca
892e82f
62c9912
6c4a312
128263f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
import { EventEmitter } from 'events'; | ||
import IORedis, { Redis } from 'ioredis'; | ||
import * as Redis from 'ioredis'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above |
||
import * as semver from 'semver'; | ||
import { load } from '../commands'; | ||
import { ConnectionOptions, RedisOptions } from '../interfaces'; | ||
import { isRedisInstance } from '../utils'; | ||
|
||
export class RedisConnection extends EventEmitter { | ||
static minimumVersion = '5.0.0'; | ||
private _client: Redis; | ||
private initializing: Promise<Redis>; | ||
private _client: Redis.Redis; | ||
private initializing: Promise<Redis.Redis>; | ||
private closing: boolean; | ||
|
||
constructor(private opts?: ConnectionOptions) { | ||
|
@@ -24,7 +24,7 @@ export class RedisConnection extends EventEmitter { | |
...opts, | ||
}; | ||
} else { | ||
this._client = <Redis>opts; | ||
this._client = <Redis.Redis>opts; | ||
} | ||
|
||
this.initializing = this.init(); | ||
|
@@ -38,7 +38,7 @@ export class RedisConnection extends EventEmitter { | |
* Waits for a redis client to be ready. | ||
* @param {Redis} redis client | ||
*/ | ||
static async waitUntilReady(client: IORedis.Redis) { | ||
static async waitUntilReady(client: Redis.Redis) { | ||
return new Promise(function(resolve, reject) { | ||
if (client.status === 'ready') { | ||
resolve(); | ||
|
@@ -60,14 +60,14 @@ export class RedisConnection extends EventEmitter { | |
}); | ||
} | ||
|
||
get client(): Promise<Redis> { | ||
get client(): Promise<Redis.Redis> { | ||
return this.initializing; | ||
} | ||
|
||
private async init() { | ||
const opts = this.opts as RedisOptions; | ||
if (!this._client) { | ||
this._client = new IORedis(opts); | ||
this._client = new Redis(opts); | ||
} | ||
|
||
await RedisConnection.waitUntilReady(this._client); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
import fs from 'fs'; | ||
import { Redis } from 'ioredis'; | ||
import path from 'path'; | ||
import * as fs from 'fs'; | ||
import * as Redis from 'ioredis'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment as above |
||
import * as path from 'path'; | ||
import { Processor, WorkerOptions } from '../interfaces'; | ||
import { QueueBase, Repeat } from './'; | ||
import { ChildPool, pool } from './child-pool'; | ||
import { Job } from './job'; | ||
import { RedisConnection } from './redis-connection'; | ||
import sandbox from './sandbox'; | ||
import { Scripts } from './scripts'; | ||
import uuid from 'uuid'; | ||
import * as uuid from 'uuid'; | ||
import { TimerManager } from './timer-manager'; | ||
import { isRedisInstance } from '../utils'; | ||
|
||
|
@@ -53,7 +53,7 @@ export class Worker<T = any> extends QueueBase { | |
|
||
this.blockingConnection = new RedisConnection( | ||
isRedisInstance(opts.connection) | ||
? (<Redis>opts.connection).duplicate() | ||
? (<Redis.Redis>opts.connection).duplicate() | ||
: opts.connection, | ||
); | ||
this.blockingConnection.on('error', this.emit.bind(this)); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ | |
*/ | ||
'use strict'; | ||
|
||
import IORedis from 'ioredis'; | ||
import * as Redis from 'ioredis'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
|
||
const path = require('path'); | ||
const util = require('util'); | ||
|
@@ -30,7 +30,7 @@ interface Command { | |
}; | ||
} | ||
|
||
export const load = async function(client: IORedis.Redis) { | ||
export const load = async function(client: Redis.Redis) { | ||
const scripts = await loadScripts(__dirname); | ||
|
||
scripts.forEach((command: Command) => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { JobsOptions } from '../interfaces'; | ||
|
||
import IORedis from 'ioredis'; | ||
import * as Redis from 'ioredis'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
import { ConnectionOptions } from './redis-options'; | ||
|
||
export enum ClientType { | ||
|
@@ -10,7 +10,7 @@ export enum ClientType { | |
|
||
export interface QueueBaseOptions { | ||
connection?: ConnectionOptions; | ||
client?: IORedis.Redis; | ||
client?: Redis.Redis; | ||
prefix?: string; // prefix for all queue keys. | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import IORedis from 'ioredis'; | ||
import * as Redis from 'ioredis'; | ||
|
||
export type RedisOptions = IORedis.RedisOptions & { | ||
export type RedisOptions = Redis.RedisOptions & { | ||
skipVersionCheck?: boolean; | ||
}; | ||
|
||
export type ConnectionOptions = RedisOptions | IORedis.Redis; | ||
export type ConnectionOptions = RedisOptions | Redis.Redis; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import { Queue, QueueEvents, Worker } from '../classes'; | ||
import { delay, removeAllQueueData } from '@src/utils'; | ||
import { expect } from 'chai'; | ||
import IORedis from 'ioredis'; | ||
import * as Redis from 'ioredis'; | ||
import { after } from 'lodash'; | ||
import { beforeEach, describe, it } from 'mocha'; | ||
import { v4 } from 'uuid'; | ||
|
@@ -21,7 +21,7 @@ describe('Cleaner', () => { | |
afterEach(async function() { | ||
await queue.close(); | ||
await queueEvents.close(); | ||
await removeAllQueueData(new IORedis(), queueName); | ||
await removeAllQueueData(new Redis(), queueName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its late now, so I am confused why we can instantiate Redis here and not just import the Redis class by this (as commented above) import { Redis } from 'ioredis' |
||
}); | ||
|
||
it('should clean an empty queue', async () => { | ||
|
@@ -127,7 +127,7 @@ describe('Cleaner', () => { | |
}); | ||
await worker.waitUntilReady(); | ||
|
||
const client = new IORedis(); | ||
const client = new Redis(); | ||
|
||
await queue.add('test', { some: 'data' }); | ||
await queue.add('test', { some: 'data' }); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can't we just do:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will work for all cases where it's used as an interface. However, this usage won't work (or at least I can't figure out how to make it work):
I'm not sure why the ioredis typings are written this way.
The import in this particular file,
src/classes/job.ts
, could be changed to this since there are no instantiations here:I used the same import style across all of the files for consistency and to account for files that need to instantiate an instance. I suppose it's a matter of opinion. I could change all of the files that don't instantiate Redis to the named import style if you'd prefer.
This reply should apply to all of your comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, but in many places we are not instantiating, just using it as an interface. Besides that, I think that we should change the name of the import to IORedis to make it explicitly that it is the module name, so it will become IORedis.Redis instead of the slightly more awkward Redis.Redis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points. I will make those changes.