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

Export Emscripten runtime for the Web #509

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Jul 24, 2019

Some runtime environments look like Node.js, but are not Node.js which confuses Emscripten loader code. I'm talking about JavaScript bundlers that are used for Web and Electron apps. These things make the code believe that it can use require() for importing and module.exports for exporting things, and expect this be the case.

However, Emscripten believes that it is smarter, detects that it's not running in Node.js environment, and just uses Module for exports. This breaks bundler's expectation and various Emscripten runtime functions (like allocate()) end up not being exported, resulting in weird TypeErrors when WasmThemis code tries calling them.

This can be avoided by exporting whatever Emscripten wants to export in Node.js style via "module.exports". We can insert this code into generated file by using --pre-js flag of emcc.

Since now we have more than one file for Emscripten compilation, move them all into a subdirectory to keep the root directory tidy.

Some runtime environments look like Node.js, but are not Node.js which
confuses Emscripten loader code. I'm talking about JavaScript bundlers
that are used for Web and Electron apps. These things make the code
believe that it can use "require()" for importing and "module.exports"
for exporting things, and expect this be the case.

However, Emscripten believes that it is smarter, detects that it's not
running in Node.js environment, and just uses "Module" for exports.
This breaks bundler's expectation and various Emscripten runtime
functions (like "allocate()") end up not being exported, resulting in
weird TypeErrors when WasmThemis code tries calling them.

This can be avoided by exporting whatever Emscripten wants to export
in Node.js style via "module.exports". We can insert this code into
generated file by using "--pre-js" flag of emcc.

Since now we have more than one file for Emscripten compilation,
move them all into a subdirectory to keep the root directory tidy.
@ilammy ilammy added the W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages label Jul 24, 2019
@ilammy ilammy requested review from vixentael and Lagovas July 24, 2019 12:42
@ilammy ilammy requested a review from shadinua as a code owner July 24, 2019 12:42
Copy link
Collaborator

@Lagovas Lagovas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
how you detect this problem?

@ilammy
Copy link
Collaborator Author

ilammy commented Jul 24, 2019

@Lagovas,

how you detect this problem?

I tried using WasmThemis in web browser and immediately ran into an issue with libthemis.js file not exporting the functions it should export. Electron apps had the same behavior (but they were fine if I told Emscripten to produce Node.js-only code).

With these changes developers can use popular tools like Browserify or webpack to bundle all JavaScript files for consumption by web browsers and it will just work (provided that libthemis.wasm file is served together with the resulting *.js file). There are many other bundlers, but these two are the most popular ones. I have tested these two, the Electron app bundler, and plain Node.js.

Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O_O

@ilammy ilammy merged commit 0b388f7 into cossacklabs:master Jul 25, 2019
@ilammy ilammy deleted the wasm-web-exports branch July 25, 2019 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants