Browserify compatibility

[arcanis]

It would be interesting to be able to run Text-Buffer in browser environments. Unfortunately, it looks impossible right now, because multiple of its dependencies actually use native node modules, and don't offer js fallbacks (even inefficient fallbacks would be fine, really).

[arcanis]

I've digged a bit, and it doesn't seem too complicated:

• All the dependencies that need to be updated to be browserify-compliant are under the Atom's organization, so you should be able to push all the required changes.
• marker-index already has a JS implementation (that seems to be tested with the same testsuite than the native code), so it should be possible to use it simply by adding "browser": "./dist/js/marker-index" in its package.json file.
• buffer-offset-index also has a JS implementation that is used to compute the expected results that the native implementation will have to return as well (so it's just as well tested as the native version). Again, it should be possible to use it simply by adding the right property into the package.json file.
• node-pathwatcher seems to be a soft dependency, in that it will not throw unless the file feature is actually used (which should never happen in a browser environment). Using a stub when browserified should work fine.

[arcanis]

I've updated my local copies of package.json to fix the issues mentioned above, and the tests apparently completed without issues (actually, some tests failed, but they also fail on a fresh clone 😞).

Would you accept a PR on each affected package with the changes discussed above?

[pablomayobre]

I would be interested to see what you have achieved, so even if not in a PR could you share your modifications?

[arcanis]

@positive07 The steps highlighted above have been the only changes required to make text-buffer work in a browser environment. It was a proof-of-concept, and I haven't yet been able to actually use it in a real-world application, but it sounds promising.

[arcanis]

Ping @maxbrunsfeld? 😃

[maxbrunsfeld]

We are interested in being able to use this module in a browser. Our preferred approach would be to use emscripten to compile the native modules to asm.js or webassembly, rather than maintaining JS ports of our native code. We don't know how hard this will be, though; it's not something we've ever tried.

[arcanis]

Hm, it would require a bit more work, since that would require to move the business logic from marker-index and buffer-offset-index into standalone C libraries, and then implement two small bridges (one using the V8 API and another using a JS API) to expose the library respectively to Node and a browser. That could work, but probably not without a fair amount of refactoring.

[maxbrunsfeld]

@arcanis All of the non-trivial logic in those modules is already in separate C++ files that don't use V8. So we'd just need to write an additional binding, using the browser APIs.

[arcanis]

I've started to experiment in this direction, starting with buffer-offset-index. Major changes are:

• my fork uses nbind instead of nan, which natively supports asm.js targets and drastically decreases the amount of C++ glue code required. Really good module all around.

• I've also had to rename most methods so that they use the same convention than in javascript (lower camelcase). It makes it quite easier to correctly export them later (otherwise I would have to remap those names somehow). Member names are still in snake case, since they are not exposed (except column and row).

• There is a tiny amount of js glue code required to support variadic parameters on a few function calls (such as passing a { row: 0, column: 0 } object instead of an actual Point instance to characterIndexToPosition), and another one to expose row and column properties as instance methods (instead of prototype accessors, which make them invisible to deepEqual and alikes). Nothing too fancy.

I haven't yet been able to test that the generated asm.js file actually works, but it compiles without issue, and it's a selling point of nbind, so I'm not overly worried. I'm going to try to apply the same changes to marker-index later tomorrow.

Feel free to let me know what you think about this.

[maxbrunsfeld]

/cc @bolinfest

[arcanis]

There's a small compatibility issue with using nbind: it doesn't yet implement automatic conversion from and to std::unordered_set and std::unordered_map (only std::vector and std::array are supported right now), which is problematic with marker-index since it is used in quite a few return values. I could probably hack my way by converting the results to vectors before returning them, but ideally this would be much better if it was supported by nbind :(

Related issue: charto/nbind#37

[arcanis]

There's also something weird going on with the asm.js build. I've improved the tests in my fork so that they are deterministic (by using a constant seed) and run on both builds, native & asm.js alike, but the asm.js tests are currently failing after 145,000 successful iterations. I have no idea what can go wrong, if it's because of nbind or Emscripten itself ... :(

[arcanis]

The error was due to a specific option that was set during the emscripten compilation (ALLOW_MEMORY_GROWTH). I've fixed it by adding a hard limit to the available memory size (134Mo). I'm not completely confident about this, since theoretically a library should be able to use as much memory as required by the user code, but apparently growing memory isn't available anymore in asm.js, and will only return with webasm.

I've opened a PR on buffer-offset-index to discuss the diff in detail.