Use Symbol.for(key) instead of Symbol(key) everywhere

[remojansen]

Expected Behavior

We should use in all our code / examples / docs (and recommend using) Symbol.for(key) to create new symbols instead of Symbol(key) because it is unsafe!

Current Behavior

We currently use in all our code / examples / docs (and recommend using) Symbol(key) to create new symbols. This is unsafe!

Steps to Reproduce (for bugs)

The following code snippet demonstrates why it is unsafe:

ts-node temp.ts 

function test1() {
    const m = new Map<symbol, number>();

    for (let i = 0; i < 50; i++) {
        const key = Symbol.for(`KEY_${i}`);
        const value = i;
        m.set(key, value);
    }

    for (let i = 0; i < 50; i++) {
        const key = Symbol.for(`KEY_${i}`);
        const value = m.get(key);
        if (value === undefined) {
            throw new Error(`Cannot get ${key.toString()}`);
        }
    }

    console.log("TEST 1 OK :)");
}

function test2() {
    const m = new Map<symbol, number>();

    for (let i = 0; i < 50; i++) {
        const key = Symbol(`KEY_${i}`);
        const value = i;
        m.set(key, value);
    }

    for (let i = 0; i < 50; i++) {
        const key = Symbol(`KEY_${i}`);
        const value = m.get(key);
        if (value === undefined) {
            throw new Error(`Cannot get ${key.toString()}`);
        }
    }

    console.log("TEST 1 OK :)");
}

test1();
test2();

Context

Using Symbol(key) is unsafe because the symbols are not unique!

From MDN about Symbol:

To create a new primitive symbol, you write Symbol() with an optional string as its description:

var sym1 = Symbol();
var sym2 = Symbol('foo');
var sym3 = Symbol('foo');

The above code creates three new symbols. Note that Symbol("foo") does not coerce the string "foo" into a symbol. It creates a new symbol each time:

Symbol('foo') === Symbol('foo'); // f

And MDN about Symbol.for(key):

The Symbol.for(key) method searches for existing symbols in a runtime-wide symbol registry with the given key and returns it if found. Otherwise a new symbol gets created in the global symbol registry with this key.

This has created a real-world issue in one of my projects.

We need PRs for all the projects:

• [x] https://github.com/inversify/InversifyJS
• [x] https://github.com/inversify/inversify-express-utils
• [x] https://github.com/inversify/inversify-express-example
• [x] https://github.com/inversify/inversify-basic-example
• [x] https://github.com/inversify/inversify-vanillajs-helpers
• [x] https://github.com/inversify/inversify-inject-decorators
• [x] https://github.com/inversify/inversify-logger-middleware
• [x] https://github.com/inversify/inversify-binding-decorators
• [x] https://github.com/inversify/inversify-restify-utils

Your Environment

• node@8.1.0
• inversify@4.5.2

Stack trace

TEST 1 OK :)

/home/rjansen/CODE/core/temp.ts:34
            throw new Error(`Cannot get ${key.toString()}`);
                  ^
Error: Cannot get Symbol(KEY_0)
    at test2 (/home/rjansen/CODE/core/temp.ts:34:19)
    at Object.<anonymous> (/home/rjansen/CODE/core/temp.ts:42:1)
    at Module._compile (module.js:569:30)
    at Module.m._compile (/usr/lib/node_modules/ts-node/src/index.ts:406:23)
    at Module._extensions..js (module.js:580:10)
    at Object.require.extensions.(anonymous function) [as .ts]
        (/usr/lib/node_modules/ts-node/src/index.ts:409:12)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)

[matthewjh]

@remojansen: what exactly is the problem with using Symbol("a")? The reason you were using symbols in the first place was to prevent collisions, no? Using Symbol.for seems little different to using strings, i.e. it collides.

[remojansen]

@matthewjh The problem is that every time you call Symbol("XXXX") you get a new unique symbol. For example, the following won't work:

container.bind<UserRepository>(Symbol("UserRepository")).to(UserRepository)

@inject(Symbol("UserRepository"))

The above doesn't work because the symbols created in the first and second call of Symbol("XXXXX") are two unique symbols.

We normally don't face this problem because we declare the symbol once in advance but I have experienced a problem in one project in which we load and unload modules dynamically and we declare symbols dynamically. Using Symbol.for("XXXXX") will always return the same symbol and not cause any problems so it is safer.

[matthewjh]

@remojansen: yes, that's right. I presumed people were only using Symbols by declaring them in advance, as you said.

But how is Symbol.for("XXXX") any different to just using "XXXX" (as a string)? I thought the only justification for using Symbols with inversify was the very fact that they're unique (no collisions), hence my confusion over the PR. :P

[remojansen]

A small application can probably work just fine with strings as IDs. As the application grows and especially if you have very large teams chances of two classes with the same name will increase and Symbol will prevent these collisions if on top of that you are doing dynamic stuff Symbol.for will protect you.

If you don't want to worry about this the best thing to do is to use Symbol.for and then you are safe for the future growth of your app.

[matthewjh]

@remojansen

How will `Symbol.for" protect you from collisions, though? If you call Symbol.for twice with the same string in the same JS context, you're going to get back the exact same symbol:

Symbol.for("a") == Symbol.for("a") // true

So I don't see how it provides any collision protection over strings. If you have a large team with two classes with the same name, and each team uses Symbol.for("CLASSNAME"), these two symbols will be identical just as if strings were used.

[matthewjh]

Or are you saying that people can easily switch from Symbol.for to Symbol as and when they encounter collisions?

[remojansen]

Yes, they should. At least I was able to do it without problems. Do you have any thoughts about why it could be a problem? Maybe I'm missing something?

[matthewjh]

@remojansen: that would work. I guess I don't see the point in using Symbol.for over a string in that case.

const TYPES = { MyType: "MyType" };

can easily be changed to

const TYPES = { MyType: Symbol("MyType") };

if uniqueness is desired and vice-versa.

Using a string is well-understood; using Symbol.for is more obscure and less clear. People may confuse why you're using Symbol.for over a string, just as I did, given their functional equivalence. Unless you're aiming for uniqueness, I would use strings. And if you are aiming for uniqueness, then use Symbol("name"). That's clearer, IMO.

[lillallol]

@remojansen

We normally don't face this problem because we declare the symbol once in advance but I have experienced a problem in one project in which we load and unload modules dynamically and we declare symbols dynamically.

Can you please provide a minimal reproducible example on that ?

Edit : Also why not use strings and go for Symbol.for() for the dynamic abstractions ? What is that thing that Symbol.for is offering you more than string ?

[penguinsAreFunny]

It really depends on how you use inversifyJS.

Symbol(string) generates unique values as Symbol.for(string) does not generate unique values but looks up the value for the given key in a registry.

Using @inject(Symbol.for(string)) is the right way if you would like to use the same string directly in your @inject-decoraters. With this solution you access your TYPES with a string. Symbol.for(X) might be called more than once with X and should always return the same value.

If you use TYPES-objects though you should use Symbol(string). With this solution you access your Types with the key stored in an object: const TYPES = { mathLib: Symbol("mathLib") // only called once. Value will stay the same in runtime }

@inject(TYPES.mathLib) // THIS IS NOT THE SAME AS: @inject(Symbol("mathLib") !!! because the second use of @inject(Symbol("mathLib") leads to another value which is not intended! container.bind...(TYPES.mathLib)

If you create another TYPES-Object and someone uses the same string like in the first TYPES you would get a collision and both TYPES have the same value. Binding two times the same value will lead to an error! const TYPES_2 = { x: Symbol("MathLib") } @inject(TYPES_2.x) - works fine, would not work with Symbol.for("MathLib")!!!

More than one TYPES-Object can occurre if you have different packages which all have TYPES to inject. This is a very common use-case!

Conclusion: The examples in the InversifyJS-documentation are wrong! When using TYPES-Object you should use Symbol and NOT Symbol.for!!! If you use a string to access your TYPES (not recommended!!!) you should use Symbol.for(string). When using Symbol.for(string) though you need to make sure that every string used is unique for the type you want to inject, which is basically not possible in larger projects and with certain modularization techniques!

[penguinsAreFunny]

A small application can probably work just fine with strings as IDs. As the application grows and especially if you have very large teams chances of two classes with the same name will increase and Symbol will prevent these collisions if on top of that you are doing dynamic stuff Symbol.for will protect you.

If you don't want to worry about this the best thing to do is to use Symbol.for and then you are safe for the future growth of your app.

I think you are wrong with this one. Please see comment above. The best way is to use Symbol combined with TYPE-Objects. I think you also want to change the InversifyJS-Documentation.

[ko22009]

We are using Symbol.for(key) bcz we have micro-frontends and we need the same identificator for it. Using Symbol(key) work only in one application, when you use like const.

[lillallol]

@ko22009 So how does using a simple string, instead of a symbol.for fail you?

[ko22009]

@ko22009 So how does using a simple string, instead of a symbol.for fail you?

no problem, but string has more memory.

[lillallol]

@ko22009 How does "a string" has more memory than Symbol.for("a string")? And lets just say that it has. Is the different that much, that is worth the confusion the usage of Symbol.for is causing?

[ko22009]

@ko22009 How does "a string" has more memory than Symbol.for("a string")? And lets just say that it has. Is the different that much, that is worth the confusion the usage of Symbol.for is causing?

doesn't matter. use it however you want