full nodejs support in file_packager.py v2

[gabrielfreire]

Continuation of #164
partially addresses #14

Changes

• where it sets the Module var Module = ..., added a check to look for process[%(EXPORT_NAME)s] which is where PyodideNode.js stores its pyodide Module.

• where it tries to set PACKAGE_PATH, added support for nodejs and stopped throwing errors if not in a browser, now the error is thrown if the user is on some mysterious environment that is not browser, worker or nodejs.

• changed the fetchRemotePackage method to check for XMLHttpRequest before trying to fetch anything, if user is not on a browser, use nodejs isomorphic-fetch/fs to fetch remote/local packages

• after the data has been fetched, there are some problematic asserts for nodejs, sometimes the buffer is not the arrayBuffer, but the arrayBuffer.buffer

• it was necessary to change from
  line 700 assert(arrayBuffer instanceof ArrayBuffer, 'bad input to processPackageData');
  to
  assert((arrayBuffer instanceof ArrayBuffer || arrayBuffer.buffer instanceof ArrayBuffer), 'bad input to processPackageData');

• and from line 701 var byteArray = new Uint8Array(arrayBuffer);
  to var byteArray = new Uint8Array(arrayBuffer instanceof ArrayBuffer ? arrayBuffer : arrayBuffer.buffer);

this format was tested using PyodideNode.js on pyodide.asm.data.js and numpy.js and works fine.

[gabrielfreire]

Also could you provide some instruction how one would use this in node? (i.e. what I need to do after launching node to get pyodide there).

The node support here is just for the data.js files generated by file_packager.py, it is the 1st phase of the integration, to get this running on node the user would need to require pyodide from PyodideNode.js inside a node js application, and this would require pyodide.js to support nodejs too, which i'm still trying to figure out how to do that without having 1000 lines pyodide.js file, my thoughts are that as the project grows we would need to separate some stuff here, pyodide.js is already kind of big and deals with a bunch of stuff, if we could separate the package loader from the language loader for example, it would be great, maybe put the fixRecursionLimit method in another Utils,js file for example.
Having some project with a structure like this:

• src/index.js
• src/packageLoader/PackageLoader.js
• src/languageLoader/LanguageLoader.js
• src/Utils/recursionTreat.js
• src/Utils/....js

the way it is structured now, pyodide.js would get even bigger and harder to maintain.
A new structure would even make the use of Typescript possible, we could build/bundle everything using Webpack and maybe publish to npm as a standalone package to fetch all webassembly related stuff remotely, no matter if you're in a browser or nodejs.

What are your thoughts about this ?

[rth]

Thanks for working on this @gabrielfreire! I'm not competent to review JS code for NodeJS, and you will have to wait for @mdboom 's review, but your work on this is very much appreciated!

i'm still trying to figure out how to do that without having 1000 lines pyodide.js file [..] the way it is structured now, pyodide.js would get even bigger and harder to maintain.

Possibly, but now it's just ~350 LoC, it's not that large, and keeping it in one file might be simpler for the now? We could always solve this issue later when it arrives?

[mdboom]

Yes, thanks for working on this! I'm sort of happy that we don't have a lot of npm / webpack stuff right now. There's already a lot of technologies that are required to understand in pyodide (Makefiles, emscripten, python packaging) that I'm a little wary of adding one more. However, I think your suggestion to provide an npm package for pyodide is a good one and will inevitably lead us down that road. Maybe we add that stuff at that time as a separate PR? I don't mind a few hundred more lines of code in pyodide.js in order to support Node now in this PR.

[gabrielfreire]

Hehe i understand you, no npm/webpack then, but is it possible to merge this PR and generate other data.js files? i can't wait to get rid of my local data.js files and be able to implement the node support fetching them remotely.

[mdboom]

How would I test that this is working in Node? (Manual testing is fine for now -- I just want to sanity check it).

[gabrielfreire]

How would I test that this is working in Node? (Manual testing is fine for now -- I just want to sanity check it).

When

We will only be able to test this in Node when i finish pyodide.js with the node integration, probably won't be for this PR as i would like to fetch the data.js files remotely and get rid of my own local ones, so, if possible, i would prefer to have the supported files generated in order to start implementation because everytime CircleCI runs a job, it pushes new files to pyodide_demo with different chunk sizes by LZ4 and i am manually having to replace the compressedData part of the code everytime, so it would be very handy to be able to fetch the full supported files remotely in order to implement/test.

How

I believe that once everything is ready in pyodide.js we need to have node installed by CircleCI.
By the use of apt-get i think you're using a Debian distribution for the image so it would be something like this in the dependencies job

sudo apt-get update
sudo apt-get install curl # maybe not needed
curl -sL https://deb.nodesource.com/setup_10.x | sudo -E bash -
sudo apt-get install -y nodejs

Remember, node.js is a different environment from Python/Web, webdriver won't cut it, so we will need a package.json file
with testing dependencies and a pyodide.test.js file containing the tests.
after having those files copied from src/ to the CircleCI image
(i believe this is what you're doing here :

build/pyodide.js: src/pyodide.js
	cp $< $@
	sed -i -e 's#{{DEPLOY}}#https://iodide.io/pyodide-demo/#g' $@

)
now in the build job run:

• npm install

and in the test job run:

• npm run tests (a script that runs all node tests)

final structure would look like this

• ./src/pyodide.js
• ./src/pyodide.test.js (this is only for node tests)
• ./package.json (to hold test dependencies)

this is how i would do it, any thoughts ?

[gabrielfreire]

I think we would have something like this in the CircleCI config.yaml

test-node:
    <<: *defaults
    steps:
      - checkout
      - attach_workspace:
          at: .
      - run:
          name: test
          command: |
            # This Debian is so old, it doesn't know about wasm as a mime type, which then
            # causes Firefox to complain when loading it.  Let's just add the new mime type.
            sudo bash -c "echo 'application/wasm wasm' >> /etc/mime.types"

            npm run tests

[gabrielfreire]

Also

Having the hiwire_get_global with the right Module.global is vital for me to keep my sanity while implementing/testing the nodejs support in pyodide.js as i can't download a new pyodide.asm.js for every CircleCI build and manually replace the code, the same goes for all the data.js files
😄

[rth]

Regarding testing in a separate job in Circle CI that totally makes sense. FYI node is already bundled under ./emsdk/emsdk/node/.

The question is more short term, to merge this PR it would be good to have some way of checking that the added changes work. I understand that other PRs may be necessary to get something fully working, but can we not check it at least roughly with some external script (e.g. what you proposed in #14 (comment) or using your external PyodideNode.js)?

(Also if you can please reference issues in PRs -- it makes finding information easier, I edited the PR description here to add the links)

[gabrielfreire]

I understand, give me 20 minutes, i'll create a new repository with all the necessary files so i don't depend on remote files being built and a test file with some tests in it

[gabrielfreire]

Hi guys @rth and @mdboom
I created this repo: https://github.com/gabrielfreire/pyodide-node
so you can easily test PyodideNode
the following is in the README.md as well

Building

Requirements

• Node v8.x +
  git clone https://github.com/gabrielfreire/pyodide-node.git
cd pyodide-node
npm install

Testing

npm test

[teonbrooks]

hey @rth, I would like to help with this PR if I can. do you remember where we left off here?

[rickyes]

ping @mdboom @gabrielfreire Hi guys, what other changes do we need if we merge this PR?

[gabrielfreire]

Hi @rickyes and @teonbrooks , this PR should have been merged 2 years ago in my opinion, but i guess this feature wasn't the focus at that moment and the PR didn't get much attention, so i kind of abandoned the project.

The repository is still there if you guys want to work on it.

Cheers.

[rickyes]

@gabrielfreire Thanks again for your contribution and reply, I'm not familiar with the implementation of this project, @teonbrooks can you help with this PR?

[rickyes]

Or I can try to familiarize myself with the project and open a new PR.

[rth]

Yes, sorry it never got merged at the time. If someone is interested in continuing it it would very welcome. In particular, it would be necessary to merge master in and add tests to the CI as discussed above. (You can create a new PR by checking-out this branch in your fork).

[teonbrooks]

@rickyes, yes, feel free to take this PR over. I initially thought I would work on it but it didn't manage to do so

[dalcde]

We have stopped using our own file_packager.py fork, so I think we ought to close this.

[rth]

Agreed. It would be good though to re-evaluate what would be necessary for nodejs support and if necessary contribute it to emscripten.