search

VulcanJS / Vulcan

🌋 A toolkit to quickly build apps with React, GraphQL & Meteor
http://vulcanjs.org
MIT License
7.98k stars 1.88k forks source link

Use Symbol.for for vulcan-accounts const STATES #2637

Closed kevinashworth closed 3 years ago

kevinashworth commented 4 years ago

Instead of Symbol(), Symbol.for() will allow global access to Symbols like STATES.SIGN_IN in the other files in a two-repo Vulcan install. This allows customization (i.e. replaceComponent) of the AccountsStateSwitcher, for example.

More info on Symbol.for() from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol:

the Symbol() function will not create a global symbol that is available in your whole codebase. To create symbols available across files ... use the method Symbol.for()...

On the other hand, if there is no use of the STATES const outside of this helpers.js file, I believe there is no harm in using the global Symbol registry anyway.

eric-burel commented 4 years ago

Interesting, I didn't know that. There is no harm either into using Symbol.for, thank you

eric-burel commented 4 years ago

Doesn't it break the way you consume them though? With Symbol.keyFor? Did you test a Vulcan app after this change?

kevinashworth commented 3 years ago

Doesn't it break the way you consume them though? With Symbol.keyFor? Did you test a Vulcan app after this change?

In my two-repo install, I've modified helpers.js as shown in this PR, then I use Symbol.for('SIGN_IN') in my own codebase. I have no need for Symbol.keyFor(). To clarify, Symbol.for() creates symbols if they don't exist or returns the appropriate symbol if it does exist, and that's all the functionality I need to modify the Sign In process. I don't know when it would be useful in Vulcan to use Symbol.keyFor() — but happy to learn of any use cases for that.

So, as far as I can tell, Symbol.for() instead of Symbol() doesn't "break the way you consume them"; it allows them to be consumed in a two-repo install. As I have been doing in my Vulcan app.

eric-burel commented 3 years ago

Thanks for the details!