Added UMD wrapper for module version

[w8r]

We are trying to use your wonderful module (thanks a lot!) as a plugin to our library. We would like to be able to use it as a drop-in add-on in the browser as well, but index.js assumes Node.js/CommonJS environment.

So I added a UMD wrapper, that simply exposes three functions according to your example. In case of plugin build it shouldn't cause andy side-effects.

I didn't re-build the library to avoid unnecessary diffs, it's better if you do it yourself in the same environment if you decide to accept the changes

Once again, thanks a lot for the great solution.

[ryanhamley]

@ChrisLoer how do you feel about this?

[ChrisLoer]

Hi @w8r really glad to hear you're getting use out of rtl-text! I would love to accommodate any packaging changes that allow it to work in other situations as long as we're confident it doesn't interfere with the current gl-js use case. Could you provide any more details about how you're using rtl-text?

[w8r]

Hi @ChrisLoer! Thanks for the response. We are using it to process RTL glyphs for a WebGL text rendering (we do graph visualisation). So ideally we wanted to add a drop-in file for the users that want to support RTL text (it's a rare case for us), and then it would be configured by our library.

The only problem is that the index.js file is not in the UMD format (cannot be added with a <script> tag), and the plugin.js assumes mapbox. So I suggested in my PR to build the index file into the UMD format.

Thanks for the consideration

[w8r]

So it wouldn't break the gl-js build, it would just make the main file more accessible.

[ChrisLoer]

🤔 Hmm, I tried this out, and it doesn't actually work on the CommonJS pathway (I was testing with just npm test, which uses index.js). It looks like the reason is that the Emscripten runtime resets the module's exports:

https://github.com/mapbox/mapbox-gl-rtl-text/blob/fa036e8e1d988de2b9b0a34cd435eb61ec2763d4/index.js#L68-L70

I didn't actually realize this was happening! 😳 But the way it worked before it would reset module.exports and then I'd add my intended exports on top of that, with the end result that on top of all the unintended internals, I also exported the things I wanted.

We'll need to figure out some way to address this. I haven't thought through what it should be yet -- I'm reluctant to go modify the emscripten runtime directly because it means I end up maintaining a semi-fork of emscripten.

[w8r]

no no, no need to change emscripten, module can be passed as the argument to the self-executing wrapper function, I believe

[ChrisLoer]

@w8r Sorry it took me a while to come back to this. I slightly modified your commits and pushed them to a PR here: https://github.com/mapbox/mapbox-gl-rtl-text/pull/14, does that look good to you?

[ChrisLoer]

Closed in favor of #14. Thanks for the contribution!

Published to NPM as 0.2.1: https://www.npmjs.com/package/@mapbox/mapbox-gl-rtl-text/v/0.2.1