define(...) and NodeJS

[IngwiePhoenix]

https://www.npmjs.com/package/amdefine

You might be interested in this since I see that Uniter still uses define(). I also am suggesting this because of NodeJS. Having it require the dist/uniter.js file loads a lot of overhead and could, in one way or another, confuse the system.

Changing index.js to use require("amdefine/intercept") and then module.exports = require("./js/main") will enable NodeJS to properly load the modules without the confusion of the browserify bundle.

[asmblah]

Hi @IngwiePhoenix,

Thanks for this, it's a very good point - the Node.js API shouldn't be including the built bundle. Ideally I'd like to remove the use of AMD throughout and convert everything over to CommonJS.

I wasn't aware of the intercept feature in amdefine, so thanks for pointing that out - looks like it hooks into all (even non-Uniter) module loads though so it might not be ideal to embed it inside the library.

I think we should either just include amdefine's prologue at the top of every module or take the plunge and convert everything over to CommonJS to keep everything simple. Not sure which is the best option to be honest but I'll definitely fix this soon!

[IngwiePhoenix]

Y’know, I have a night to spend. So, I am going to fork and convert. :) Uniter is awesome and I am planning to use it in another project of mine - so I might as well contribute to it!

[IngwiePhoenix]

I'm almost done. Oh my god Uniter is HUGE....respect for having worked all that logic on your own. Its gigantic o-o.

I am only missing some stuff in the grammar/ folder, and then i have everything converted. Just gotta runt ests and see if I converted correctly :)

[IngwiePhoenix]

Just a small handful left.

So...many...files. And so long paths. But it has a structure that is not to be ignored. I'll PR you the update when i'm done. :)

[asmblah]

Thanks very much @IngwiePhoenix, I look forward to seeing it!

[asmblah]

Just FYI, I've pushed a few fixes/features I've been working on to master that you might want to merge into your branch - I'm hoping it won't cause you too much trouble merging in.

Thanks again!

[IngwiePhoenix]

Well the most I have been doing is modifying the top and bottom of each file to turn each define() statement into separate require()’s and instead of returning, the values are module.exports’ed. I hope I can merge that indeed o.o...

[IngwiePhoenix]

I am done with the conversion of the current state and having not merged current master.

When I try to run Uniter now, I get an absolutely empty result. Like, i am using this:

var php = require('./index').createEngine('PHP');
php.getStdout().on('data', function (text) { console.log(text); });
php.getStderr().on('data', function (text) { console.log(text); });
php.execute('<?php print "Hello from PHP!";');

and there is no output on the conosle. But using th eunified source... works. So there clearly is something wrong.

What is the best way to debug this? I can push my current state to my fork's master if you want to review it yourself.

[asmblah]

Hi @IngwiePhoenix, thanks very much for your effort here. It's difficult to say what might be causing the issue, and debugging features are a little lacking at the moment, sorry - so if you could push up your changes as you say (might make sense to push to a branch other than master in your repo, but entirely up to you) then I'll take a look :)

[IngwiePhoenix]

Oops! I had a derp with my branching and now it did end up on master. Ahh...whatever. :) https://github.com/IngwiePhoenix/uniter/commit/42be07b6f8c6620a013510152f545bc0596f416d

[asmblah]

Thanks @IngwiePhoenix - wow, it's difficult to see what's changed in that diff :)

It would probably be a bit of a pain now, but I wonder whether it would be easier if we didn't 'outdent' the main body of each module for now, so the diff only covered the imports and exports sections of each module.

The tests should be handy checking the conversion hasn't broken anything - but they seem to still be in AMD format rather than CommonJS. Maybe that would help debug the issue?

Cheers

[IngwiePhoenix]

I use an editor that shows me what will be seen as new in a diff...so when I began shifting, it already showed me that even without undenting the lines, all of them were marked. So I just didn't really look for a fix x)

I'll see if the tests bring up any result. But if you happen to come across soemthing, let me know! :)

[asmblah]

Only a few small changes to get it working so I'll just post them here:

$ git diff languages/
diff --git a/languages/PHP/interpreter.js b/languages/PHP/interpreter.js
index 1b4f607..d683176 100644
--- a/languages/PHP/interpreter.js
+++ b/languages/PHP/interpreter.js
@@ -12,7 +12,7 @@
  */

 'use strict';
-var util = require('./util'),
+var util = require('../../js/util'),
     Call = require('./interpreter/Call'),
     Exception = require('../../js/Exception/Exception'),
     KeyValuePair = require('./interpreter/KeyValuePair'),
diff --git a/languages/PHP/interpreter/builtin/functions/array.js b/languages/PHP/interpreter/builtin/functions/array.js
index c81ae7e..0560bdb 100644
--- a/languages/PHP/interpreter/builtin/functions/array.js
+++ b/languages/PHP/interpreter/builtin/functions/array.js
@@ -9,7 +9,7 @@

 'use strict';

-var utils = require('../../../../../js/util'),
+var util = require('../../../../../js/util'),
     PHPError = require('../../../interpreter/Error'),
     Variable = require('../../../interpreter/Variable');

diff --git a/languages/PHP/interpreter/builtin/functions/string.js b/languages/PHP/interpreter/builtin/functions/string.js
index 37a5a3b..968e15e 100644
--- a/languages/PHP/interpreter/builtin/functions/string.js
+++ b/languages/PHP/interpreter/builtin/functions/string.js
@@ -9,7 +9,7 @@

 'use strict';

-var utils = require('../../../../../js/util'),
+var util = require('../../../../../js/util'),
     PHPError = require('../../../interpreter/Error'),
     Variable = require('../../../interpreter/Variable');

diff --git a/languages/PHP/interpreter/builtin/functions/variableHandling.js b/languages/PHP/interpreter/builtin/functions/variableHandling.js
index c198be3..1f2a4f3 100644
--- a/languages/PHP/interpreter/builtin/functions/variableHandling.js
+++ b/languages/PHP/interpreter/builtin/functions/variableHandling.js
@@ -9,7 +9,7 @@

 'use strict';

-var utils = require('../../../../../js/util'),
+var util = require('../../../../../js/util'),
     PHPError = require('../../../interpreter/Error'),
     Variable = require('../../../interpreter/Variable');```

[asmblah]

Mochify invokes the UnAMDify transform via Browserify, so you can run the tests in their current state that way: npm run test-browser

[IngwiePhoenix]

This is now solved through everything in @uniter