Skip to content

Conversation

@kripken
Copy link
Member

@kripken kripken commented Aug 30, 2019

Without this, no amount of work would let the user override these at runtime. For example, we set a readBinary property to guard against incorrect usages of that changed API, but in binaryen there happens to be an added export by that name - so in ASSERTIONS builds we hit an error.

With this change, projects still get the assertion check, but they can overwrite the property manually, then set it, as a workaround, with something like this:

Object.defineProperty(Module, 'readBinary', { writable: true });
Module.readBinary = ..new value..;

… amount of work would let the user override these at runtime. For example, we set a readBinary property to guard against incorrect usages of that changed API, but in binaryen there happens to be an added export by that name - so in ASSERTIONS builds we hit an error. With this change, projects still get the assertion check, but they can delete the property manually, then set it, as a workaround.
@kripken kripken changed the title Use writable properties in our assertion checks Use configurable properties in our assertion checks Aug 30, 2019
@kripken kripken merged commit ff22fba into incoming Sep 19, 2019
@kripken kripken deleted the config branch September 19, 2019 21:14
kripken added a commit to WebAssembly/binaryen that referenced this pull request Dec 6, 2019
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
…9360)

Without this, no amount of work would let the user override these at runtime. For example, we set a readBinary property to guard against incorrect usages of that changed API, but in binaryen there happens to be an added export by that name - so in ASSERTIONS builds we hit an error.

With this change, projects still get the assertion check, but they can overwrite the property manually, then set it, as a workaround, with something like this:

Object.defineProperty(Module, 'readBinary', { writable: true });
Module.readBinary = ..new value..;
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.

3 participants