Skip to content

Conversation

mcasimir
Copy link
Collaborator

@mcasimir mcasimir commented Jun 30, 2020

export const shellApiType = Symbol('...') is working fine in node js with npm run start but in the binary we get actually 2 distinct unique symbols: one used in the class definition and one used in the shell evaluator.

To fix this we use Symbol.for('...') with a unique name.

@mmarcon
Copy link
Member

mmarcon commented Jul 1, 2020

Tested on macOS, works well.

Copy link
Contributor

@lrlna lrlna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoa what a tiny bug that causes so many problems! really nice catch ✨

@lrlna
Copy link
Contributor

lrlna commented Jul 1, 2020

I am confused why the regular windows tests are running here -- i am sure we removed that task in master.

@lrlna lrlna merged commit 51d968c into master Jul 1, 2020
@lrlna lrlna deleted the MONGOSH-276 branch July 1, 2020 08:36
@aherlihy
Copy link
Contributor

aherlihy commented Jul 2, 2020

Ah good catch!! Sorry I missed this - I tested with the binary before I made the final switch of adding the second symbol, but going forward will always test with the binary as a final step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants