Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modules Are Empty Objects #271

Open
pixelshaded opened this issue Jun 19, 2014 · 12 comments
Open

Modules Are Empty Objects #271

pixelshaded opened this issue Jun 19, 2014 · 12 comments

Comments

@pixelshaded
Copy link

The script tags:

<script src="~/Scripts/curl.js"></script>
<script src="~/Areas/Analytics/Scripts/Charting/ChartingMain.js"></script>

ChartingMain:

curl({ baseUrl: "/" }, ["Areas/Analytics/Scripts/Charting/ChartingApp"], function (ChartingApp) {
    var chartingApp = new ChartingApp();
});

ChartingApp:

define([
"require", 
"exports",
"Areas/Analytics/Scripts/Charting/ChartViewModel",
"Areas/Analytics/Scripts/Routing/RouteManager",
"Areas/Analytics/Scripts/Charting/FusionCharts/DualYCombination2D",
"Areas/Analytics/Scripts/Charting/FusionCharts/Column2D",
"Areas/Analytics/Scripts/Charting/KendoDataViz/DataVizConfigGenerator",
"Areas/Analytics/Scripts/Charting/KendoDataViz/DataVizRenderer",
"Areas/Analytics/Scripts/Charting/FusionCharts/FusionChartRenderer"
], function(
require,
exports,
ChartViewModel,
RouteManagerImpl,
DualYCombination2D,
Column2D,
DataVizConfigGenerator,
DataVizRenderer,
FusionChartRenderer
) { 
//some code
});

If I look in the network tab of the chrome debugger, I can see that the charting app modules are getting loaded. When I put break points in the modules, they get hit and return. The majority of modules return constructor functions, however when the ChartingApp define callback runs, all the modules in the callback are just base Objects with no parameters.

I was previously using requirejs to load these modules and everything worked. What might I be missing here?

@unscriptable
Copy link
Member

Hey @pixelshaded,

Unfortunately, curl.js has timing problems with that particular flavor of AMD format. Specifically, the exports CommonJS local variable. It works great for trivial examples or for resolving the occasional circular dependency, but fails on non-trivial dependency graphs.

Here are some options:

  1. Continue to use RequireJS for now, but keep an eye on modern, ES6-style loaders such as RaveJS and/or SystemJS and jump to one of those when you feel comfortable. You'll be migrating to one of these in the future, anyways.
  2. Use curl.js's cjsm11 loader. You would have to strip the AMD boilerplate from your modules -- leaving pure CommonJS -- for this to work. Also, it looks like you're not organizing your modules into packages or using proper module names. You seem to be using urls everywhere ("Areas/Analytics/Scripts/...")? A proper "packages" configuration is required to use a curl loader. However, we may be able to make this work with an undocumented feature in curl.js.
  3. Convert your modules so they don't require "exports" or use exports. If modules return their value from the factory instead, curl.js won't have problems.

Do any of those options look interesting?

-- John

@pixelshaded
Copy link
Author

I'm not actually using exports or require modules in the modules themselves. The javascript files are generated using the Typescript compiler (this is what is adding the exports and require dependency). So I'm not actually using or following the CommonJS module pattern.

As for the module names, these are meant to look more like namespaced classes in a c language, specifically because almost all my modules return constructor functions. I make all these module paths relative to the root directory, ideally following this same pattern across the entire project so that using the same modules in different places would have the same module name (since many examples have the module names relative to the main.js file that require js loads which could be anywhere depending on the page).

So my intention is certainly to use pure AMD modules. Any resemblance to CommonJS comes from the Typescript compiler. Because of this, I don't really have much flexibility as far as how these modules are formatted. I could tell the compiler to generate CommonJS modules, but I'm writing modules for the client so that doesn't make much sense.

@unscriptable
Copy link
Member

I make all these module paths relative to the root directory, ideally following this same pattern across the entire project so that using the same modules in different places would have the same module name

Yes. paths == urls, essentially. I don't understand how using full paths/urls fixes anything. To me, there's no difference between "Areas/Analytics/Scripts/Charting/FusionCharts/FusionChartRenderer" and "FusionCharts/FusionChartRenderer" where "FusionCharts" has been configured to be at "Areas/Analytics/Scripts/Charting/FusionCharts". Except, of course, that "FusionCharts/FusionChartRenderer" is much shorter and more pleasing to my eyes. :)

I could tell the compiler to generate CommonJS modules

This sounds like it might be exactly what you want. If everything under the "Areas" directory is TypeScript-generated CommonJS, then configuring curl to use cjsm11 on that package* would probably work. e.g.:

curl.config({
    packages: {
        Areas: { location: "", config: { loader: "curl/loader/cjsm11" } },
        curl: { location: "????" }
    }
});

Hm.... I couldn't complete the curl package config because it looks like you copied only the curl.js file, not the rest of the modules. :(

  • not actually a package :)

@pixelshaded
Copy link
Author

These modules aren't commonjs modules pretending to be AMD modules. I dont use exports or require in them. I'm not seeing any reason to have the compiler make them commonjs modules, especially because im not writing modules for server code.

Full paths make the code more maintainable for two reasons:
Firstly, when looking at a module, the dependencies clearly show where they are located. This is a reason why in .NET you want your namespaces to match your folder structure. This makes things less confusing. Secondly, using the root as your base URL standardizes the path for that module, regardless of the entry point (since its not relative to the entry point). This makes refactoring easier if your path changes since its the same everywhere in your code base (search and replace magic).

Im not sure I follow your comment on not actually being a package. What makes it a package? I copied the entry point module and the main application module. The chartingApp depends on a bunch of other modules, but Im not sure how showing the code for those helps.

@unscriptable
Copy link
Member

I dont use exports or require in them.

Surely, the TypeScript compiler assigns named values to exports or a single value to module.exports. I can't see how TypeScript-to-CommonJS transpiled modules could work otherwise. ???

Sorry I got pedantic about packages. Packages are supposed to make your code easy to refactor without search-and-replace -- but perhaps that's a discussion for a different place and time. :) As long as there's a directory (or directories) where we can say "everything under this directory is CommonJS", we could possibly make this work.

@pixelshaded
Copy link
Author

Here is a simple class in typescript with the resulting AMD and CommonJS module.

/// <reference path="../../../../Scripts/typings/jquery/jquery.d.ts" />
import Routes = require("Areas/Analytics/Scripts/Routing/Routes");
import RouteConfig = require("Areas/Analytics/Scripts/Routing/RouteConfig");
import IRouteManager = require("Areas/Analytics/Scripts/Routing/IRouteManager");

class RouteManager implements IRouteManager {
    public routes: Routes;

    constructor(config: RouteConfig) {

        if (config.routeUrl && config.callback) {
            $.ajax({
                url: config.routeUrl,
                type: 'GET',
                contentType: 'application/json; charset=utf-8',
                success: (data: Routes) => {
                    this.routes = data;
                },
                complete: function () {
                    config.callback();
                }
            });
        } else if (config.routes) {
            this.routes = config.routes;
        }
    }
}
export = RouteManager;

Here is the resulting AMD module

define(["require", "exports"], function(require, exports) {
    var RouteManager = (function () {
        function RouteManager(config) {
            var _this = this;
            if (config.routeUrl && config.callback) {
                $.ajax({
                    url: config.routeUrl,
                    type: 'GET',
                    contentType: 'application/json; charset=utf-8',
                    success: function (data) {
                        _this.routes = data;
                    },
                    complete: function () {
                        config.callback();
                    }
                });
            } else if (config.routes) {
                this.routes = config.routes;
            }
        }
        return RouteManager;
    })();

    return RouteManager;
});

Here is the resulting CommonJS module

 var RouteManager = (function () {
    function RouteManager(config) {
        var _this = this;
        if (config.routeUrl && config.callback) {
            $.ajax({
                url: config.routeUrl,
                type: 'GET',
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    _this.routes = data;
                },
                complete: function () {
                    config.callback();
                }
            });
        } else if (config.routes) {
            this.routes = config.routes;
        }
    }
    return RouteManager;
})();
module.exports = RouteManager;

As you can see, the resulting AMD module always includes exports and require modules because of the compiler. I don't actually use them in the module code itself.

@unscriptable
Copy link
Member

It's unfortunate that the TypeScript compiler injects ["require", "exports"] even if your modules don't require those.

That CommonJS module looks perfectly suited for consumption by the cjsm11 loader, though. :) Let us know if you decide to try it.

@pixelshaded
Copy link
Author

So because of require and exports, curl is unable to load my modules properly, even though I dont use them?

@pixelshaded
Copy link
Author

Is this related? #265

@unscriptable
Copy link
Member

Hey @pixelshaded,

Yes, I think that is the same issue. You can try the dev branch. It may solve the problem. Note, though, that I noticed a timing issue in Chrome with the dev branch when mixing the legacy loader (curl/loader/legacy). All other tests seem to pass, atm.

@gliheng
Copy link

gliheng commented Oct 27, 2014

I ran into the very same situation today using typescript compiler. What's the status of this issue?

@kshutkin
Copy link

I think that fix in the dev branch not fixes this issue, but I was able to use curl with Typescript.

Minimal fix is the next one:

// track exports
def.exportsReady = function executeFactory (deps) {
    when(isPreload || preload, function () {
        // only resolve early if we also use exports (to avoid
        // circular dependencies). def.exports will have already
        // been set by the getDeps loop before we get here.
        if (def.exports) {
            execute(deps);
            if (def.exports['__esModule']) {
                def.progress(msgFactoryExecuted);
            }
        }
    });
};

In my project it plays nice but I'm not sure that it is the best fix.

Warning: it breaks normal modules with exports unless you add magic __esModule: true property to them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants