espruino / EspruinoTools

JavaScript library of tools for Espruino - used for the Web IDE, CLI, etc.
Apache License 2.0
150 stars 89 forks source link

Module dependancies can be in the wrong order #110

Closed ChristiaanWillemsen closed 4 years ago

ChristiaanWillemsen commented 4 years ago

I think It's possible to have the module dependancies in the wrong order. I fixed this by doing the folloing in modules.js

 var loadProcessedModule = function(module) {
      Promise.all(newPromises).then(function() {
        // add the module to the beginning of our array
        if (Espruino.Config.MODULE_AS_FUNCTION)
          loadedModuleData.push(
            "Modules.addCached(" +
              JSON.stringify(module.name) +
              ",function(){" +
              module.code +
              "});"
          );
        else
          loadedModuleData.push(
            "Modules.addCached(" +
              JSON.stringify(module.name) +
              "," +
              JSON.stringify(module.code) +
              ");"
          );
        // if we needed to load something, wait until we have all promises complete before resolving our promise!

        resolve();
      });
    };
gfwilliams commented 4 years ago

Do you have example of JS code that'd cause this to break? I haven't heard of any cases where modules weren't found or were in the wrong order so I'm concerned about trying to fix something that isn't broken! :)

ChristiaanWillemsen commented 4 years ago

Hi there.

Well, I’ve used typescript to generate the js. In this I have one TS class A using another B, where A is used in the main application file. This resolves into A first, then B. While the order needs to be the reverse.

I can try to make the code I have a bit more manageable to you can try yourself… Let me come back to you on that.

On 5 Nov 2019, at 11:09, Gordon Williams notifications@github.com wrote:

Do you have example of JS code that'd cause this to break? I haven't heard of any cases where modules weren't found or were in the wrong order so I'm concerned about trying to fix something that isn't broken! :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/espruino/EspruinoTools/issues/110?email_source=notifications&email_token=ACAMDDBSS5VNICH4M4NCOETQSFA6PA5CNFSM4JJAENEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDCJTZA#issuecomment-549755364, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACAMDDHPZXXCMLYYG23CJPDQSFA6PANCNFSM4JJAENEA.

gfwilliams commented 4 years ago

Ahh, ok, got you - just did a simple test with:

// foo.js
var a = require("bar");
print(a);
// bar.js
exports = require("baz").bob;
// baz.js
exports.bob=42;

and got...

Modules.addCached("bar","exports=require('baz').bob;");
Modules.addCached("baz","exports.bob=42;");
var a = require("bar");
print(a);

I just applied your changes - thanks!