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 triggers override mistake when overriding O.p.hasOwnProperty by assignment #3075

Closed
erights opened this issue Feb 10, 2021 · 6 comments
Closed
Labels
bug For bugs or other software errors

Comments

@erights
Copy link
Contributor

erights commented Feb 10, 2021

At docs/vega-core.js line 32463 and at doc/vega.js line 45238 appears the line

  exports.hasOwnProperty = has;

This exports object inherits from Object.prototype which has a method named hasOwnProperty. Unfortunately, because of the EcmaScript Override Mistake (also here and here), in running in an environment (like SES, XS, or TC53 JavaScript for Embedded Systems) where Object.prototype is frozen, the Override Mistake will cause that assignment to fail.

From the line numbers above, it is clear these two files are generated files. However I do not understand what the actual source lines are that are translated into these generated lines, so I'm not sure what the best fix is. Two immediate possibilities:

  • Have exports not inherit from Object.prototype. Depending on the role of exports in the translation, I would not be surprised if that is not feasible.
  • Use defineProperty rather than assignment, such as
     Object.defineProperty(exports, 'hasOwnProperty', {
         value: has, 
         enumerable: true, 
         writable: true, 
         configurable: true 
     });

See Agoric/agoric-sdk#2373 for the consequences of this bug in the Agoric use of SES.

@erights
Copy link
Contributor Author

erights commented Mar 4, 2021

See Agoric/agoric-sdk#2575

@domoritz
Copy link
Member

domoritz commented Mar 4, 2021

I suspect the problem is in https://github.com/vega/vega/blob/master/packages/vega-util/src/hasOwnProperty.js. Do you have an idea how we can resolve the issue you are seeing?

@erights
Copy link
Contributor Author

erights commented Mar 5, 2021

No, that's not the problem. There is nothing about that file that would trigger the override mistake.

My first post above #3075 (comment) explains the problem well, but complains about the wrong file. The problem is in vega-util/build/vega-util.js . Aside from that, please ask about anything unclear in the explanation in that first post.

Vega actually has three problems along these lines, for constructor, 'hasOwnPropertyandtoString`, all in the vega-util.js file. A patch which fixes all these is at
https://github.com/Agoric/agoric-sdk/blob/master/patches/vega-util%2B1.16.0.patch

@erights
Copy link
Contributor Author

erights commented Mar 5, 2021

I just looked again at your sources rather than my patch. It looks like my patch is against, not a source file, but a file built from those sources. Is that correct? If so, then my first post was correct. The problem is in docs/vega-code.js and docs/vega.js .
However, both of these are huge files. Much larger than I'd expect from a source file. But the string exports.hasOwnProperty appears in your source tree in these two files and no where else. Where does this text come from?

@erights
Copy link
Contributor Author

erights commented Mar 5, 2021

See #3109

@jheer
Copy link
Member

jheer commented Mar 16, 2021

Closed via #3109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For bugs or other software errors
Projects
None yet
Development

No branches or pull requests

3 participants