Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent the get trap for the proxy from taking effect if called with internal Symbols or internal 'inspect" prop key #3

Merged

Conversation

alexkolson
Copy link
Contributor

@alexkolson alexkolson commented Oct 3, 2019

turns out when you call console.log(proxiedThing) node calls the get trap with built in Symbols
In the throw TypeError statement we subsequently have issues converting the Symbol to a string value implicitly.
I opted to just check if the propertty is a symbol or if its the inspect internal string and if it is just short circuit the get trap.

See also: nodejs/node#10731

turns out when you call "console.log(proxiedThing)" node calls the get trap with built in Symbols
In the throw TypeError we subsequently have issues converting the Symbol to a string value implicitly.
I opted to just check if the propertty is a symbol or if its the inspect internal string and if it is just short circuit the get trap
@alexkolson alexkolson changed the title fixes #1 Prevent the get trap for the proxy from taking effect if called with internal Symbols or internal 'inspect" prop key Oct 3, 2019
@alexbepple alexbepple mentioned this pull request Oct 28, 2019
src/create-type.test.js Outdated Show resolved Hide resolved
@alexbepple
Copy link
Owner

What is the reason for the changes to yarn.lock?
What is the reason for seemingly unrelated changes, e.g. newline at eof in package.json?
If these changes are truly unrelated, let's get rid of them.

Currently, yarn lint fails. (Yes, I know, CI is not set up. Sorry.)

Do you want to work on the PR or shall I just make the changes myself?

@alexkolson
Copy link
Contributor Author

@alexbepple done

@alexbepple alexbepple merged commit 6b8269e into alexbepple:master Nov 28, 2019
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.

2 participants