tannerntannern / ts-mixer

A small TypeScript library that provides tolerable Mixin functionality.
MIT License
379 stars 27 forks source link

class is wok, but if run with jest will throw error #22

Closed bluelovers closed 3 years ago

bluelovers commented 4 years ago
import LRU from '../';

describe('clear() sets the cache to its initial state', () => {

    console.dir(LRU)

});
  ● Test suite failed to run

    TypeError: Object prototype may only be an Object or null: undefined
        at Function.create (<anonymous>)

      at Object.exports.hardMixProtos (node_modules/ts-mixer/dist/util.js:54:31)
      at Object.Mixin (node_modules/ts-mixer/dist/mixins.js:16:18)
tannerntannern commented 4 years ago

I'm sorry @bluelovers, but this doesn't give me nearly enough information to reproduce the issue. Please provide a minimal reproducible example.

bluelovers commented 4 years ago

need run with jest

module.exports = {
    clearMocks: true,
    moduleFileExtensions: ['ts', 'js'],
    testEnvironment: 'node',
    //testMatch: ['**/*.test.ts', '**/*.spec.ts'],
    testRegex: ['\\.(test|spec)\\.(ts|tsx)$'],
    //testRunner: 'jest-circus/runner',
    transform: {
        '^.+\\.ts$': 'ts-jest',
    },
    verbose: true,
    collectCoverage: true,
}
import { Mixin, mix } from 'ts-mixer';

import QuickLRU from 'quick-lru';
import { EventEmitter } from 'events';

const LRU = Mixin(QuickLRU, EventEmitter)

it('should not throw', () => {
    console.dir(LRU)
})

  ● Test suite failed to run

    TypeError: Object prototype may only be an Object or null: undefined
        at Function.create (<anonymous>)

       8 | import { EventEmitter } from 'events';
       9 | 
    > 10 | const LRU = Mixin(QuickLRU, EventEmitter)
         |             ^
      11 | 
      12 | it('should not throw', () => {
      13 |  console.dir(LRU)

      at Object.exports.hardMixProtos (node_modules/ts-mixer/dist/util.js:54:31)
      at Object.Mixin (node_modules/ts-mixer/dist/mixins.js:16:18)
      at Object.<anonymous> (test/temp.spec.ts:10:13)
tannerntannern commented 4 years ago

@bluelovers, I am unable to reproduce this issue. I used your code sample in my test suite and no errors were thrown. I was using mocha instead of jest, but the test framework shouldn't matter for an issue like this. I am closing the issue for now, but if you have clearer instructions on how to reproduce I'd be happy to help.

steveetm commented 3 years ago

This is really a jest only issue, struggling with same problem atm. Instructions are clear, repro is plain simple, just try to mixin something into a class extended from EventEmitter and run the test with jest.

steveetm commented 3 years ago
/**
 * Identifies the nearest ancestor common to all the given objects in their prototype chains.  For most unrelated
 * objects, this function should return Object.prototype.
 */
export const nearestCommonProto = (...objs: object[]): Function => {
    if (objs.length === 0) return undefined;

    let commonProto = undefined;
    const protoChains = objs.map(obj => protoChain(obj));

    while (protoChains.every(protoChain => protoChain.length > 0)) {
        const protos = protoChains.map(protoChain => protoChain.pop());
        const potentialCommonProto = protos[0];

        if (protos.every(proto => proto === potentialCommonProto))
            commonProto = potentialCommonProto;
        else
            break;
    }

    return commonProto;
};

If you return with Object instead of undefined then everything is fine, but there is a test which especially tests this function to not return undefined, while the comment says it should return Object.prototype for most unrelated objects. Probably some jest magic in the bakcground, but I don't see while this function should ever return undefined. :/

tannerntannern commented 3 years ago

Thanks for digging into this @steveetm. It does seem very strange that this only affects Jest, but I will have to look into this more. Also interesting that the issue comes down to this function. I haven't had time to investigate yet, but I would think that something needs to change in the handling of the result of this function rather than changing the function itself.

I think undefined makes perfect sense for special situations. What is the common prototype between two objects? Probably Object.prototype, but not necessarily. Object.create(null) has no prototype, so to ask what prototype it has in common with something else is nonsense -- hence undefined. (Although I could maybe see an argument for null instead). Also the zero objects case: What's the common object between an empty set of objects? Again, nonsense question, so undefined.

tannerntannern commented 3 years ago

Hi @steveetm, I just released v5.4.0 to resolve a different issue. Curious if this issue still exists for you with the new version? I didn't explicitly have this issue in mind when working, but there were a few changes related to enabling strict null checks that makes me wonder whether this issue will resolve itself.

tannerntannern commented 3 years ago

Closing due to inactivity. I will reopen if folks are still running into this issue with the new version.