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

Vega (used in stat-logger) overrides O.p.hasOwnProperty by assignment #2373

Closed
erights opened this issue Feb 10, 2021 · 4 comments · Fixed by #6854
Closed

Vega (used in stat-logger) overrides O.p.hasOwnProperty by assignment #2373

erights opened this issue Feb 10, 2021 · 4 comments · Fixed by #6854
Assignees
Labels
bug Something isn't working

Comments

@erights
Copy link
Member

erights commented Feb 10, 2021

Our mitigation of the override mistake adds 8 junk lines to every display of every object by the VSCode debugger's object inspector. #2324 would enable to stop mitigating Object.prototype.constructor and thereby remove two of these. If we could stop mitigating Object.prototype.hasOwnProperties we could remove another two. Together, this would be a significant noise reduction.

@FUDCo AFAICT Object.prototype.hasOwnProperty is overridden by assignment only by something called "vega-util" which I think is only linked in due to some stats library you're pulling in. https://github.com/vega/vega-util itself claims to be superceded by https://github.com/vega/vega , so it is possible that upgrading that dependency might painlessly make the problem go away.

@erights erights added the bug Something isn't working label Feb 10, 2021
@FUDCo
Copy link
Contributor

FUDCo commented Feb 10, 2021

It's probably good to keep up with updates to libraries we use, but I'm willing to bet you a nickel that upgrading will not fix this. How would I go about testing if the newer version of vega is still broken or not?

@erights
Copy link
Member Author

erights commented Feb 10, 2021

In your local copy of SES-shim, comment out the listing of hasOwnProperty at https://github.com/Agoric/SES-shim/blob/bf06c3a4c97991b7377eba08af049c6d2ea64d31/packages/ses/src/enablements.js#L62

Locally link an agoric-sdk to that modified local copy of the SES-shim.

Then, I think it fails under the normal yarn test. If it doesn't, let's try to reproduce this together.

@erights
Copy link
Member Author

erights commented Feb 10, 2021

but I'm willing to bet you a nickel that upgrading will not fix this

You are correct. See vega/vega#3075 . I owe you a nickel. Would you accept $0.05 worth of bitcoin instead?

@FUDCo FUDCo changed the title Something we're pulling in seems to override O.p.hasOwnProperty by assignment Vega (used in stat-logger) overrides O.p.hasOwnProperty by assignment Feb 17, 2021
@erights
Copy link
Member Author

erights commented Dec 24, 2022

Hi @FUDCo See vega/vega#3109 . I think this means

  • we can make this problem go away by upgrading to the latest vega
  • my nickel debt to you is cancelled
  • you now owe me a nickel

I'll accept it in IST

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants