-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
This does merge and all tests (nodejs / browser) via |
I think the breakages is probably related to this #5 |
@jacogr good point this will INDEED break some things but it is not even merged yet. I assume you are using the version in npmjs, right? |
More-or-less. I use a heavily modified version from npmjs. So use that as the base and then apply my fixes/cleanups to the wrappers. |
This is now ready to be pulled but an ideal scenario is to have the new version of the original schnorrkel published to fix this: |
# var wasm = get_wasm() | ||
# // this line is a gamble... | ||
# setTimeout(() => wasm = get_wasm(), 500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller needs to wait for the actual promise result to ensure it has been added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed but then the export functions will be broken (export
can only be used in the top level code.). Webpack's async import()
function should be a proper fix for this I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will defintaly not merge this branch until at least this ugly thing is gone but it is good to have it pushed in here so that it can be tested without the need to place this horrble piece of code in npm :D
to fix #2