browserifying forge

[mvhenten]

Hi,

After strugging for quite some time, I've come to realize that browserify does not play well with the AMD support built into forge. I want to use browserify as part of my tool chain for some particular reasons - getting things to work in webworkers is one of them.

While reading the sources, I figured out how to rewrite the parts I need using esprima. This entails rewriting the sourcecode significantly automatically.

While playing with esprima, it prove to work a lot better then I had anticipated. I've published some of the work to Github, but I'd like to have your consent in doing so, especialy for publishing this to npmjs.

Curious for your thoughts, and many thanks for your work...

https://github.com/mvhenten/browserify-forge-aes-crypt

[dlongley]

Hi @mvhenten,

So long as you comply with the terms of the license (simply include the copyright information, etc.) then you're free to use the source as you like. However, it would be great if you could contribute back to the project somehow. I realize that this might be too difficult. But maybe you could do a PR with a script that can be run to modify forge and build a bundle that would work with browserify. It's ok if the script doesn't pull in all of forge right away (it only does AES and dependencies); it can be a work in progress. That way people can use the same package to work with both build systems.

JFYI, the current plan is to eventually replace the (horribly ugly) boilerplate we have in each file with ES6-module syntax combined with some decent ES6-module loader shim that can generate boilerplate depending on whatever legacy build/loading system people prefer to use today.

[mvhenten]

Yeah, I realized that the copyright notices were stripped too, I'll look into preserving them.

I'd love to contribute back, unfortunately, the conversion is only 95% automatic, the remaining 5% is human intervention - very small but hard to automate. I've kind of become obsessed with esprima, so I'll definately ping back if I get it working.

My output currently are bare node modules, there's no more forge object being passed around, so it's delegating dependency management to the node module system ( this is what browserify uses too) and removes a lot of boilerplate.

Thanks, I'll let you know and I'll make sure to comply then :)

On Mon, Jun 16, 2014 at 5:04 PM, Dave Longley notifications@github.com wrote:

Hi @mvhenten https://github.com/mvhenten,

So long as you comply with the terms of the license (simply include the copyright information, etc.) then you're free to use the source as you like. However, it would be great if you could contribute back to the project somehow. I realize that this might be too difficult. But maybe you could do a PR with a script that can be run to modify forge and build a bundle that would work with browserify. It's ok if the script doesn't pull in all of forge right away (it only does AES and dependencies); it can be a work in progress. That way people can use the same package to work with both build systems.

JFYI, the current plan is to eventually replace the (horribly ugly) boilerplate we have in each file with ES6-module syntax combined with some decent ES6-module loader shim that can generate boilerplate depending on whatever legacy build/loading system people prefer to use today.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46190091.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

:)

[mvhenten]

Sooo I had some fun and made the import fully automized - including the preservation of the license. I'm not sure where to take it from here - I can keep the module in sync with the main repository in a somewhat sustainable way. I have yet to test actual browser integration ( the browserify module compiles but need to look into randomness ).

As a proof-of-concept I've ran a full import of the codebase. The files that follow the "initModule" pattern can all be parsed, thus stripped from the AMD snippets, converted into "real" module.exports... Unfortunately, all comments are stripped too and I have no idea how to test this fully.

[dlongley]

So does that mean you have a simple script that can be run to produce a forge browserify module? If so, could you do a PR with that script and some instructions in the README for running it? If the script works as you say, there's no need for an extra repository, right? People can just clone the main repo (or install from npm) and then run the script if they want to use forge w/browserify. It will be a little unorthodox, but I think it will be temporary and better than having two different npm modules.

Ultimately, what I expect we'll want to do (at some point) is the mirror image of that process, so forge, when installed from npm will work immediately with node or browsersify, and then you can run some browserify/grunt tools to convert it to AMD/whatever other module API you need (for example, see: Browserify and UMD). And maybe we'll just have a post install hook w/npm to generate something that works for everyone.

[mvhenten]

Well, it's not simple :)

A browserify module is "just" an NPM module, so this module needs to be published to npm independently etc. The issue with the current NPM module is that browserify cannot determine the required requires since they're code generated, so the forge browserified forge module basically contains only the toplevel forge code.

I'll make a subdir called something like "forge-browserify-aes" ( says what it does on the tin ) including package.json and all the rest and send it in as a pull request. Still, it needs to be published to npm independently.

The good news is that I got some of the relevant tests ported from the node.js module as well. As I expected, the pain is in the "random" utility, so I'll be looking in to that next.

On Tue, Jun 17, 2014 at 5:55 PM, Dave Longley notifications@github.com wrote:

So does that mean you have a simple script that can be run to produce a forge browserify module? If so, could you do a PR with that script and some instructions in the README for running it? If the script works as you say, there's no need for an extra repository, right? People can just clone the main repo (or install from npm) and then run the script if they want to use forge w/browserify. It will be a little unorthodox, but I think it will be temporary and better than having two different npm modules.

Ultimately, what I expect we'll want to do (at some point) is the mirror image of that process, so forge, when installed from npm will work immediately with node or browsersify, and then you can run some browserify/grunt tools to convert it to AMD/whatever other module API you need (for example, see: Browserify and UMD http://dontkry.com/posts/code/browserify-and-the-universal-module-definition.html). And maybe we'll just have a post install hook w/npm to generate something that works for everyone.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46326699.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

Well, what I was thinking was that we could look into potentially switching forge over to being compatible with browserify and then use a build system to generate the AMD/UMD boilerplate for the files for those that need it. So, in the end, anyone using node.js or browserify would just install forge from npm and use it -- and anyone that needed another API would run a script and then go from there.

While that would be disruptive to those who currently use AMD, it seems like that approach would be the least disruptive in the long term if we want to have browserify support (which is probably a good idea).

[mvhenten]

I'm not too knowledgable on AMD, but I think that could be very feasable.

I'll try and do the pr after the match tonight so you can have a look at the code produced by esprima, and see if I can port a couple more tests.

On Wed, Jun 18, 2014 at 4:16 PM, Dave Longley notifications@github.com wrote:

Well, what I was thinking was that we could look into potentially switching forge over to being compatible with browserify and then use a build system to generate the AMD/UMD boilerplate for the files for those that need it. So, in the end, anyone using node.js or browserify would just install forge from npm and use it -- and anyone that needed another API would run a script and then go from there.

While that would be disruptive to those who currently use AMD, it seems like that approach would be the least disruptive in the long term if we want to have browserify support (which is probably a good idea).

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46440883.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

@mvhenten, ok, well I would suggest that you don't put too much effort in ... as it could be that it would be fairly easy to just manually change forge to run with browserify (remove all the boilerplate that is causing a problem with browserify) and then add build scripts that will rewrite the files for other module loaders. With that approach, we probably don't need something like esprima, right? Maybe I'm missing something else that it's bringing to the table.

[mvhenten]

it won't be much effort - it's basically what I've done already :)

Thing is, there's a reasonable amount of logic dealing with dependency loading and attaching things to the forge object. This is all gone after the transform - e.g. there's no more forge object being passed around.

There is a small for this: it'll make it easier to create modular builds ( like lodash does ) if you don't need all of forge.

On Wed, Jun 18, 2014 at 4:58 PM, Dave Longley notifications@github.com wrote:

@mvhenten https://github.com/mvhenten, ok, well I would suggest that you don't put too much effort in ... as it could be that it would be fairly easy to just manually change forge (remove all the boilerplate that is causing a problem with browserify) to run with browserify and then add build scripts that will rewrite the files for other module loaders.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46446785.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

There is a small for this: it'll make it easier to create modular builds ( like lodash does ) if you don't need all of forge.

Well, just to be clear, you can do that now, by requiring only those components that you want to use. The forge object that is passed around doesn't have to be a global and will only have those APIs attached to it that are in the dependency chain. If you require/include forge directly you get all of it, but you can just include components.

[mvhenten]

Yes, you are right. I've just been reading into AMD a little more. Never saw a use for it as I've moved to node.js quite early and never really needed async module loading ( on mobile it's better to do less request then more). So does the AMD system for forge also intends to support loading the library in multiple requests as they are needed? Still reading the code but havent' figured it out yet.

On Wed, Jun 18, 2014 at 8:26 PM, Dave Longley notifications@github.com wrote:

There is a small for this: it'll make it easier to create modular builds ( like lodash does ) if you don't need all of forge.

Well, just to be clear, you can do that now, by requiring only those components that you want to use. The forge object that is passed around doesn't have to be a global and will only have those APIs attached to it that are in the dependency chain. If you require/include forge directly you get all of it, but you can just include components.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46474300.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

So does the AMD system for forge also intends to support loading the library in multiple requests as they are needed?

Yes, it already does and we don't want to lose the AMD API. However, if we want to add browserify support, what we'll need to do is generate that AMD API using a tool (there are various that are part of or tie into browserify) such that it ends up behaving the same way existing users of the library expect it to. The only difference for them will be that subsequent versions of forge will require them to run an extra script to generate the AMD API ... or, as mentioned above, we could have an npm post-install hook do it automatically and just point to a different directory for the AMD/UMD version.

That will give us browserify+node support and AMD support. What changes is that the AMD/UMD boilerplate is generated instead of being checked into the repo. We may have to do some path fiddling or notify AMD consumers of forge that some paths have changed, but otherwise, I think we can support all of the popular module loading mechanisms with this approach.

[mvhenten]

Well, if I can help out here... I'm willing to do so, but I think we'll have to discuss this a little in detail. My first take on this is what you can see in the pull request - basically making everything straight forward requires.

A little bit extra work is involved in moving the requires back up to the top and into var statements. From that, re-adding the AMD wrapping code is trivial (especially using Esprima to help out) There is a little bit of leak here and there where functions are attached directly onto the forge object I think, but overall...

I think it won't be flawless, but it's worth a try and fun to hack on.

basically transpiling the code into "straight" requires, and then transpiling those back into AMD modules etc. This Could Be Automated. If i can make it go back and forth, the test suite should not substantially break or anything. Famous Last Words.

I'll have a look :)

On Wed, Jun 18, 2014 at 11:32 PM, Dave Longley notifications@github.com wrote:

So does the AMD system for forge also intends to support loading the library in multiple requests as they are needed?

Yes, it already does and we don't want to lose the AMD API. However, if we want to add browserify support, what we'll need to do is generate that AMD API using a tool (there are various that are part of or tie into browserify) such that it ends up behaving the same way existing users of the library expect it to. The only difference for them will be that subsequent versions of forge will require them to run an extra script to generate the AMD API ... or, as mentioned above, we could have an npm post-install hook do it automatically and just point to a different directory for the AMD/UMD version.

That will give us browserify+node support and AMD support. What changes is that the AMD/UMD boilerplate is generated instead of being checked into the repo. We may have to do some path fiddling or notify AMD consumers of forge that some paths have changed, but otherwise, I think we can support all of the popular module loading mechanisms with this approach.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46497177.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

Well, I was thinking we could just manually change the code over to use requires -- and then follow this: Browserify and UMD to auto-convert to AMD/UMD/whatever.

[mvhenten]

love the undertitle.

How about running that "import-all" script and see what is left? I ported nodejs/test/aes.js in minutes after that. My biggest concern currently is the random module, I'm not sure I did that one well.

I may have some time tonight to have a look at this.

On Thu, Jun 19, 2014 at 4:22 PM, Dave Longley notifications@github.com wrote:

Well, I was thinking we'd just manually change the code over to use requires -- and then follow this: Browserify and UMD http://dontkry.com/posts/code/browserify-and-the-universal-module-definition.html to create scripts to convert to AMD/UMD/whatever.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46566272.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

Well, my understanding is that the script removes all the documentation/comments, doesn't it? If so, and that can't be remedied, we'll want to just manually fix the files to use requires ... and then add the necessary grunt support to re-output AMD/UMD, etc. Sorry if that means there was some amount of wasted effort on the script. :(

Ultimately, since the plan is to change the main files now ... we want to keep all comments and documentation and just make it work with node.js and browserify. Once that's working, we can pull in grunt and make sure everything still works with AMD/UMD/etc. We shouldn't have to change any of the tests to accomplish this... we can still run those as is -- since they use node.js and phantomjs to run and currently work fine. We'll just need to run the AMD/UMD script (again, maybe via npm post-install hook) before running the tests. That may mean we need a whole new directory of files that are output for the AMD API, but that's fine, we'll just redirect the test suite and notify AMD users of the library that the paths changed.

[mvhenten]

Not at all, the script was fun - and I wasn't aware on your plans to rewire amd :)

I was thinking of having "one more look" indeed - see if I can preserve documentation or add it back manually using some simple methods. OTOH, I don't know your code as well as you do of course - so I don't exactly know where to cut deeply.

Still, at the end of the day, it's propably boring work and more error prone - as both test and code needs porting you're fighting a battle on two fronts. From studying the test in the nodejs directory, I get the idea that those tests are a good starting point.

Would it be an idea to have an attempt at porting a small part into nodejs/lib, and make relevant tests work with that part as a first trial? (classical refactoring) Once a small part is done, put it on a branch, pull request, and evaluate the result..?

I don't have loads of time the coming few days but this could be feasable for me tonight (I'm on CEST)

On Thu, Jun 19, 2014 at 4:57 PM, Dave Longley notifications@github.com wrote:

Well, my understanding is that the script removes all the documentation/comments, doesn't it? If so, and that can't be remedied, we'll want to just manually fix the files to use requires ... and then add the necessary grunt support to re-output AMD/UMD, etc. Sorry if that means there was some amount of wasted effort on the script. :(

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46571228.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

Not at all, the script was fun - and I wasn't aware on your plans to rewire amd :)

Well, we are always interested in making forge work with other popular tools -- so long as it's not too difficult to do so :). So if we can make it load with just about any of the popular JS module loaders, that would be great.

I was thinking of having "one more look" indeed - see if I can preserve documentation or add it back manually using some simple methods. OTOH, I don't know your code as well as you do of course - so I don't exactly know where to cut deeply.

It may be better to use the output of your tool as a side-by-side comparison while more surgically going in and making manual updates to get forge loading w/browserify.

Would it be an idea to have an attempt at porting a small part into nodejs/lib, and make relevant tests work with that part as a first trial?

Ok, here's the plan:

I started a 'browserify' branch and moved all files from 'js' to 'lib'. I added an npm postinstall hook that currently just copies the files from 'lib' to 'js'. I changed the package.json file to set the main file to 'lib/forge.js', so this is the one that will be loaded by require() via node or browserify. This approach should allow us to add support for browserify but also keep everything the same for anyone using UMD (so long as they install from npm).

Here are the next steps:

1. Get a grunt build system working that will convert non-UMD files to UMD files. Add a script to convert a list of files from 'lib' and overwrite what is in 'js' (so this runs after the copy operation).
2. Manually remove the UMD boilerplate from the files in forge/lib and add requires where necessary. We can maybe use the output of script you've written to do a side-by-side comparison to help with debugging.
3. For each file that is being worked on, add it to the list of files that will be converted via the build system and that will overwrite what's in 'js'.
4. Don't modify the tests at all -- just run them after converting each file until they pass. This ensures the tests run properly in both node and with phantomjs via AMD.
5. Once all the files are working, remove the copy from the npm postinstall and just use the build system to do the conversion of everything in 'lib'.

Hopefully I didn't miss anything in there, but I think this should get us browserify support and essentially keep things the same for any UMD users that install via npm.

[mvhenten]

One thing I missed -

Switching to browseriy/npm module approach would entail that none of the modules extend a common object anymore - on node that would be considered "monkeypatching" ( You can do it, but there's nog guarantee on the state of the object you're extending from the point of require - or the process currently running it - they all have different copies)

The top-level forge module whould then require the sub-level files ( so on node you would always "get the whole snake" ). Run-time loading is not something desireable on server-side ( less predictable ). As far as node's concerned, the code is compiled at start, so there's little overhead in performance ( couple of extra bytes in memory).

The tests in the nodejs directory are different from the ones in test right? Those tests could be used to validate the require interface, while the ones in test are full-stack.

That would open up the opportunity of first doing the require module loading, then adding the build system in place.

IMHO That would give the best of both ends...

On Thu, Jun 19, 2014 at 7:14 PM, Dave Longley notifications@github.com wrote:

Not at all, the script was fun - and I wasn't aware on your plans to rewire amd :)

Well, we are always interested in making forge work with other popular tools -- so long as it's not too difficult to do so :). So if we can make it load with just about any of the popular JS module loaders, that would be great.

I was thinking of having "one more look" indeed - see if I can preserve documentation or add it back manually using some simple methods. OTOH, I don't know your code as well as you do of course - so I don't exactly know where to cut deeply.

It may be better to use the output of your tool as a side-by-side comparison while more surgically going in and making manual updates to get forge loading w/browserify.

Would it be an idea to have an attempt at porting a small part into nodejs/lib, and make relevant tests work with that part as a first trial?

Ok, here's the plan:

I started a 'browserify' branch and moved all files from 'js' to 'lib'. I added an npm postinstall hook that currently just copies the files from 'lib' to 'js'. I changed the package.json file to set the main file to 'lib/forge.js', so this is the one that will be loaded by require() via node or browserify. This approach should allow us to add support for browserify but also keep everything the same for anyone using UMD (so long as they install from npm).

Here are the next steps:

1.

Get a grunt build system working that will convert non-UMD files to UMD files. Add a script to convert a list of files from 'lib' and overwrite what is in 'js' (so this runs after the copy operation). 2.

Manually remove the UMD boilerplate from the files in forge/lib and add requires where necessary. We can maybe use the output of script you've written to do a side-by-side comparison to help with debugging. 3.

For each file that is being worked on, add it to the list of files that will be converted via the build system and that will overwrite what's in 'js'. 4.

Don't modify the tests at all -- just run them after converting each file until they pass. This ensures the tests run properly in both node and with phantomjs via AMD. 5.

Once all the files are working, remove the copy from the npm postinstall and just use the build system to do the conversion of everything in 'lib'.

Hopefully I didn't miss anything in there, but I think this should get us browserify support and essentially keep things the same for any UMD users that install via npm.

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46589359.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[dlongley]

Yeah, that's fine that you just get the whole lib if you use node. That's, I presume, how it's primarily used now anyway -- and what I was expecting. We will want to ensure we can still get it piecemeal in the UMD-generated code, though.

The tests in the nodejs directory are different from the ones in test right? Those tests could be used to validate the require interface, while the ones in test are full-stack.

The tests in the nodejs directory are run both by node and by phantomjs. The directory is probably poorly named -- that name arose because the original test suite was based on a python server and manually run (and quite old). We plan on completely removing the old suite and moving the "nodejs" directory to just something like "test" or "tests".

In the nodejs/test directory, there is a special test called "browser.js" that will start the node server that then uses phantomjs to test the AMD-loaded, client-side modules. All of the tests in that directory are run in node ... it's just that the browser.js test will start up a server which will then fetch all of the tests again (except browser.js) and run them with phantomjs. This way all of the tests are run twice, once in each environment: node + browser (phantomjs).

[mvhenten]

Ok all clear then :)

Something came up for tonight - social obligations. but there's always tomorrow.

On Thu, Jun 19, 2014 at 7:39 PM, Dave Longley notifications@github.com wrote:

Yeah, that's fine that you just get the whole lib if you use node, that's, I presume, how it's primarily used now anyway -- and what I was expecting. We will want to ensure we can still get it piecemeal in the UMD-generated code, though.

The tests in the nodejs directory are different from the ones in test right? Those tests could be used to validate the require interface, while the ones in test are full-stack.

The tests in the nodejs directory are run both by node and by phantomjs. The directory is probably poorly named -- that name arose because the original test suite was based on a python server and manually run (and quite old). We plan on completely removing the old suite and moving the "nodejs" directory to just something like "test" or "tests".

In the nodejs/test directory, there is a special test called "browser.js" that will start the node server that then uses phantomjs to test the AMD-loaded, client-side modules. All of the tests in that directory are run in node ... it's just that the browser.js test will start up a server which will then fetch all of the tests again (except browser.js) and run them with phantomjs. This way all of the tests are run twice, once in each environment: node + browser (phantomjs).

— Reply to this email directly or view it on GitHub https://github.com/digitalbazaar/forge/issues/126#issuecomment-46592322.

webdeveloper & consultant @technischepartij.nl Mobile: +31 (0)630841238

Retiefstraat 51-III 1092VX Amsterdam The Netherlands

[mvhenten]

Hey, reporting back after a weekend hiatus - I see some changes in there - won't have lots of time on my hand coming week but I'll keep an eye and get back in touch when I have time to help out. (soccer is also messing with my time these days)

cheers and later.

[dlongley]

Addressed by #456.