inversify / InversifyJS

A powerful and lightweight inversion of control container for JavaScript & Node.js apps powered by TypeScript.
http://inversify.io/
MIT License
11.02k stars 712 forks source link

regression: v6 no longer handles singletons with value `undefined` #1518

Open simhnna opened 1 year ago

simhnna commented 1 year ago

This code works without errors using inversify 5.1 but fails in inversify 6

import { Container } from "inversify";

const container = new Container();
const s = Symbol('foo');
container.bind(s).toConstantValue(undefined);

console.log(container.get(s));

container.unbindAll();

produces:

/project/node_modules/inversify/lib/container/container.js:453
        var constructor = Object.getPrototypeOf(instance).constructor;
                                 ^

TypeError: Cannot convert undefined or null to object
    at Function.getPrototypeOf (<anonymous>)
    at Container._deactivate (/project/node_modules/inversify/lib/container/container.js:453:34)
    at Container._deactivateIfSingleton (/project/node_modules/inversify/lib/container/container.js:634:21)
    at Container._deactivateSingletons (/project/node_modules/inversify/lib/container/container.js:639:31)
    at /project/node_modules/inversify/lib/container/container.js:287:19
    at /project/node_modules/inversify/lib/container/lookup.js:113:13
    at Map.forEach (<anonymous>)
    at Lookup.traverse (/project/node_modules/inversify/lib/container/lookup.js:112:19)
    at Container.unbindAll (/project/node_modules/inversify/lib/container/container.js:286:33)
    at file:///project/inversify-bug.mjs:10:11
simhnna commented 1 year ago

Actually primitive types apparently also cause issues. Using a number instead of undefined results in the following stacktrace

/project/node_modules/inversify/lib/container/container.js:467
            throw new Error(ERROR_MSGS.ON_DEACTIVATION_ERROR(constructor.name, ex.message));
                  ^

Error: onDeactivation() error in class Number: Reflect.hasMetadata is not a function
    at Container._deactivate (/project/node_modules/inversify/lib/container/container.js:467:19)
    at Container._deactivateIfSingleton (/project/node_modules/inversify/lib/container/container.js:634:21)
    at Container._deactivateSingletons (/project/node_modules/inversify/lib/container/container.js:639:31)
    at /project/node_modules/inversify/lib/container/container.js:287:19
    at /project/node_modules/inversify/lib/container/lookup.js:113:13
    at Map.forEach (<anonymous>)
    at Lookup.traverse (/project/node_modules/inversify/lib/container/lookup.js:112:19)
    at Container.unbindAll (/project/node_modules/inversify/lib/container/container.js:286:33)
    at file:///project/inversify-bug.mjs:10:11
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
simhnna commented 10 months ago

I'm happy to contribute to make this work again if it's intended to work with primitive types. If it's not it would be nice to state that somewhere

PodaruDragos commented 10 months ago

yes please, PR welcome

Jameskmonger commented 7 months ago

Added to milestone 6.0.3, we will aim to resolve this regression as a priority. cc @PodaruDragos