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

Return entire module as promise #108

Merged
merged 1 commit into from
Oct 4, 2017
Merged

Return entire module as promise #108

merged 1 commit into from
Oct 4, 2017

Conversation

buu700
Copy link
Contributor

@buu700 buu700 commented Oct 4, 2017

As discussed in #105.

@jedisct1
Copy link
Owner

jedisct1 commented Oct 4, 2017

Awesome!

@jedisct1 jedisct1 merged commit 571b212 into jedisct1:master Oct 4, 2017
@buu700
Copy link
Contributor Author

buu700 commented Oct 4, 2017

Oh, sorry, just noticed I forgot to remove line 565 (thought I'd done that before committing).

@jedisct1
Copy link
Owner

jedisct1 commented Oct 4, 2017

No problem. Removed :)

@jedisct1
Copy link
Owner

jedisct1 commented Oct 5, 2017

Aie. It doesn't seem to work, at least without wasm.

> require('libsodium-wrappers')
{ add: [Function],
  base64_variants:
   { ORIGINAL: 1,
     ORIGINAL_NO_PADDING: 3,
     URLSAFE: 5,
     URLSAFE_NO_PADDING: 7 },
  compare: [Function],
  from_base64: [Function],
  from_hex: [Function],
  from_string: [Function: a],
  increment: [Function],
  is_zero: [Function] }

Not a promise :(

@jedisct1
Copy link
Owner

jedisct1 commented Oct 5, 2017

And with wasm, the promise is rejected

ReferenceError: libsodiumModule is not defined

@jedisct1
Copy link
Owner

jedisct1 commented Oct 5, 2017

Don't bother if this looks hard to fix, we can stick to the previous system with the promise returned along with other properties.

@jedisct1
Copy link
Owner

jedisct1 commented Oct 5, 2017

00e8ed0 probably fixes the second thing :)

@jedisct1
Copy link
Owner

jedisct1 commented Oct 5, 2017

Now without wasm we get all the symbols. But directly, instead of the promise.

buu700 added a commit to buu700/libsodium.js that referenced this pull request Oct 5, 2017
@buu700
Copy link
Contributor Author

buu700 commented Oct 5, 2017

Oops, sorry about the libsodiumModule oversight. Not exactly sure why it would return everything directly in the non-wasm case, but #109 might fix that.

jedisct1 added a commit that referenced this pull request Oct 5, 2017
buu700 added a commit to buu700/libsodium.js that referenced this pull request Oct 5, 2017
jedisct1 added a commit that referenced this pull request Oct 5, 2017
buu700 added a commit to buu700/libsodium.js that referenced this pull request Oct 5, 2017
jedisct1 added a commit that referenced this pull request Oct 5, 2017
This reverts commit c41454d.

Revert "#108 fix"

This reverts commit 76016bc.

Revert "libsodiumModule -> libsodium"

This reverts commit 00e8ed0.

Revert "Unexport ready"

This reverts commit bb0b50d.

Revert "return entire module as promise"

This reverts commit d262e55.
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