Split NodeJS and browser setup

[dapplion]

Should be merged with herumi/bls#65

---

Splits module initialization logic from BLS wrapper logic

• bls.js setup logic is reduced to 3 lines using the createBlsModule function exported by bls_c.js. If Inline wasm file for easier cross-platform setup bls#65 is merged createBlsModule returns an already initialized Module with the WASM source thanks to inlining.
• The NodeJS and browser specific logic is splited into index.js for NodeJS and bls-demo.js. External library consumers can use index.js just as they did before with bls.js. The bls-demo.html demo site works as before.

[herumi]

Thank you for the PR.
But your patch does not pass the test https://travis-ci.org/github/herumi/bls-eth-wasm/builds/739669483 .
Did the way of the test been changed?

[herumi]

I'm sorry that I missed herumi/bls#65 . I'll check it again.

[dapplion]

I'm sorry that I missed herumi/bls#65 . I'll check it again.

@herumi This PR assumes that the wasm file is inlined in bls_c.js otherwise the test would fail. I would recommend merging herumi/bls#65 first and then this one

[dapplion]

@herumi I've rebased this branch & PR on your dev branch after you've merged herumi/bls#65. Now you can run

node test.js

and it should work fine

[herumi]

I have one question.
Is it standard way to require not bls.js but index.js?
This modification breaks backward compatibility.

[dapplion]

I have one question.
Is it standard way to require not bls.js but index.js?
This modification breaks backward compatibility.

Yes, it is standard for index.js to be the entry point of a library. I also created index.js to prevent an unnecessary big diff in this PR.

[herumi]

Many thanks. I have a fews day off now. I'll check it later.

[dapplion]

Many thanks. I have a fews day off now. I'll check it later.

I've pushed a commit to prevent breaking changes. If you prefer to have index.js as the entry point you can revert the last commit. Let me know if you have any more questions

[herumi]

Your advice is very helpful. Many thanks.
I use index.js instead of bls.js.

[herumi]

May I move setup for browser into bls.js?
e9130b9

If it is not the standard way, then I'll revert it.

[dapplion]

May I move setup for browser into bls.js?
e9130b9

If it is not the standard way, then I'll revert it.

Usually, web developers use bundling tools such as webpack which bundle all javascript code into a single file. In a similar fashion to emcc inlining the wasm file to bls_c.js. So usually the standard way is to not have code to inject global state the browser global window object. However, for the sake of simplicity in your .HTML example adding browser window injection code prevents having to add webpack to the project. I think is fine for now, I can do another PR afterwards showing you the webpack setup for a browser demo

[herumi]

I'm now studying webpack and babel, etc. Please wait for a moment.

[dapplion]

I'm now studying webpack and babel, etc. Please wait for a moment.

If you need help in your study with a direct chat, please don't hesitate to say hi at our discord server https://discord.gg/QpMMT6Z3

[herumi]

@dapplion
Thank you for helping me.

I'm sorry that your discord url is expored.

I'm studying webpack and writing a sample code.
https://github.com/herumi/misc/tree/master/webpack/test2

In test2/src,

make makes add_c.js and node test.js runs well.
But in test2/,

make causes an error.

ERROR in ./src/add_c.js 71:20-43
Module not found: Error: Can't resolve 'path' in '/home/shigeo/Program/misc/webpack/test2/src'

if (ENVIRONMENT_IS_NODE) {
 if (ENVIRONMENT_IS_WORKER) {
  scriptDirectory = require("path").dirname(scriptDirectory) + "/";
 } else {
  scriptDirectory = __dirname + "/";
 }
 read_ = function shell_read(filename, binary) {
  var ret = tryParseAsDataURI(filename);
  if (ret) {
   return binary ? ret : ret.toString();
  }
  if (!nodeFS) nodeFS = require("fs");
  if (!nodePath) nodePath = require("path");
  filename = nodePath["normalize"](filename);
  return nodeFS["readFileSync"](filename, binary ? null : "utf8");
...

in src/add_c.js, require is not called, so I comment out these require, then
make makes test2/index.js but node test-webpack.js causes an error.

Could you give me some advice?

[dapplion]

@herumi The discord invite URL is public, don't worry.

EMCC adds nodejs code which webpack fails to resolve because it's targeting the browser webpack-contrib/css-loader#447. To fix it set the target to node

// webpack.config.js
module.exports = {
  ...
  target: "node",
};

Note: If you name the config file webpack.config.js it will be detected automatically without needing the --config option.

[herumi]

That's very kind of you.
I can run a sample for Node.js and brower.
https://github.com/herumi/misc/blob/master/webpack/test2/webpack.config.js
But I have to change the target for them. Can I make index.js which supports both Node.js and browser like the current bls.js?

[herumi]

I modified:

• blsSetupFactory can be local 6523eb0
• rename createBlsModule to blsCreateModule 3457afd

I tried bls-eth-wasm in React, then bls is not visible.
So I added window.bls = bls 08dacb9
I think that it is bad way. Is there any good way?

[dapplion]

That's very kind of you.
I can run a sample for Node.js and brower.
https://github.com/herumi/misc/blob/master/webpack/test2/webpack.config.js
But I have to change the target for them. Can I make index.js which supports both Node.js and browser like the current bls.js?

You as developer of the library won't use webpack to distribute the library. Instead, consumers of the library will use webpack, mainly front-end developers. I would structure the repo as

/
├── src
│   ├── bls.js
│   └── index.js
├── browser-demo
│   ├── demo.html
│   ├── demo.js
│   └── webpack.config.js
└── nodejs-demo
    └── demo.js

So in case someone wants to use webpack for NodeJS they can use a different webpack configuration

[dapplion]

I modified:

• blsSetupFactory can be local 6523eb0
• rename createBlsModule to blsCreateModule 3457afd

I tried bls-eth-wasm in React, then bls is not visible.
So I added window.bls = bls 08dacb9
I think that it is bad way. Is there any good way?

I've created a setup with webpack with your browser demo: PR #7. It uses vanilla js, but I can add React if you want to see how that would work.

[herumi]

It sounds good.
I wanted a static HTML demo, so I modified it a bit.

[dapplion]

It sounds good.
I wanted a static HTML demo, so I modified it a bit.

I have to say adding <script>window.module = {}</script> at the top of the static HTML file is very creative.

However, you can showcase your library on Github pages https://pages.github.com/ which would allow you greater flexibility. Such as splitting bls.js in more than one file, or writing it with Typescipt

For the browser-demo, committing the package-lock.json is recommended. You can read more about it here https://docs.npmjs.com/cli/v6/configuring-npm/package-lock-json

[herumi]

Thank you for all the advice.

Such as splitting bls.js in more than one file, or writing it with Typescipt

You are right. But I'm not a frontend engineer and I have many tasks and I can't spend a lot of time to study it.
Please wait a moment until I study TypeScript.

For the browser-demo, committing the package-lock.json is recommended.

I see.

[herumi]

I've removed EXPORT_NAME option because it seems unnecessary to create a module.
9cdd2ef#diff-3e2513390df543315686d7c85bd901ca9256268970032298815d2f893a9f0685

[dapplion]

While EXPORT_NAME is not mandatory it's use is recommended in the emcc docs. Since it's available in the global window object there can be naming collisions if someone uses more than one wasm factories within the same website

[herumi]

Thanks for your advice.
As you say, I think that it is better to avoid global naming collisions and I do not know the drawback without EXPORT_NAME now, so I'll do without EXPORT_NAME unless some problem happens.