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

Proposal: Refactor bundled --outFile result #6419

Closed
SetTrend opened this issue Jan 9, 2016 · 8 comments
Closed

Proposal: Refactor bundled --outFile result #6419

SetTrend opened this issue Jan 9, 2016 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@SetTrend
Copy link

SetTrend commented Jan 9, 2016

The solution suggested in #6382 doesn't work as expected. It creates false define() statements in the JavaScript file:

importerror

I believe the TypeScript compiler doesn't react correctly in regard to bundled files.

IMHO, import { module1.Identifier2, module2.Identifier1 module3.Identifier1 } from "./moduleFile" should be the only valid syntax here.

That syntax should result in loading the bundle file and import all the required modules (which I suppose should become namespaces in the generated, i.e. referenced, bundled, JavaScript file) and identifiers from there.

@SetTrend
Copy link
Author

SetTrend commented Jan 9, 2016

Converting module syntax to namespace syntax in the bundle file feels reasonable to me, because the file gets loaded completely anyway. To me it seems unnecessary to retain the costly module syntax/infrastructure in a bundle file.

I don't feel it will be a feasible measure of leaving this issue to be required to add a whole bunch of aliases to a requirejs or similar configuration. Imagine this: "Download Angular2 from here! Hey, and don't forget to add these 30+ below aliases to your module loader configuration for it to work:"

@SetTrend SetTrend changed the title Cannot reference bundled --outFile result Proposal: Refactor bundled --outFile result Jan 9, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 11, 2016

What does "Refactor bundled --outFile result" mean? can you provide more details.

@SetTrend
Copy link
Author

Yes, sure. Lets have a look at the small example project taken from SetTrend/TS18 ...

This is the current output generated by TSC using --outFile --module amd --target ES5 (I enhanced the output here with an external reference to jQuery):

var __extends = (this && this.__extends) || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
define("_Base", ["require", "exports"], function (require, exports) {
    "use strict";
    var _Base = (function () {
        function _Base(element) {
            this.element = element;
            this._var = 1;
        }
        return _Base;
    }());
    Object.defineProperty(exports, "__esModule", { value: true });
    exports.default = _Base;
});
define("Derived", ["require", "exports", "_Base"], function (require, exports, _Base_1) {
    "use strict";
    var Derived = (function (_super) {
        __extends(Derived, _super);
        function Derived(element) {
            _super.call(this, element);
        }
        return Derived;
    }(_Base_1.default));
    Object.defineProperty(exports, "__esModule", { value: true });
    exports.default = Derived;
});
define("Manager", ["require", "exports", "jquery"], function (require, exports, $) {
    "use strict";
    var Manager = (function () {
        function Manager(provider) {
            this._provider = provider;
            $("body").addClass("loaded");
        }
        return Manager;
    }());
    Object.defineProperty(exports, "__esModule", { value: true });
    exports.default = Manager;
});

//# sourceMappingURL=all.js.map

In this simple example we find three defines. For each of these defines we need to create a configuration entry in the loader's configuration file or none of the defines will be found.

As the all.js file is loaded and interpreted in one large piece anyway, it is of no advantage to have each of the sub modules declare their own define.

I suggest to merge them all into one single standard define and export each of the sub-modules as namespaces, i.e. properties of the default output:

"use strict";

define(["require", "exports", "jquery"]
      ,  function (require, exports, $)
        {  exports.default =
          {  _Base: function (element)    // no closures => no enclosurement
                    {
                      this.element = element;
                      this._var = 1;
                    }

          ,  Derived: function (__super)
                      {
                        return __extends(function(element) { __super.call(this, element); }
                                        , __super
                                        );
                      }(this._Base)

          ,  Manager: function (provider)    // no closures => no enclosurement
                      {
                        this._provider = provider;

                        $("body").addClass("loaded");
                      }
          }
        });



var __extends = (this && this.__extends)
                || function (d, b)
                    {
                      for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
                      function __() { this.constructor = d; }

                      d.prototype = b === null
                        ? Object.create(b)
                        : (__.prototype = b.prototype, new __());

                      return d;
                    };

//# sourceMappingURL=all.js.map

While the current `import` statements result in the following JavaScript:
define(["require", "exports", "Derived", "Manager"], function (require, exports, Derived_1, Manager_1) {
    "use strict";
    var d = new Derived_1.default(null);
    var m = new Manager_1.default(d);
});

//# sourceMappingURL=main.js.map

The suggested import to look like this: import myLib from "./tests/Scripts/all.js" and rather generate something similar to this:

"use strict";

define(["require", "exports", "all"]
      , function (require, exports, all)
        {
          var d = new all.default.Derived(null);
          var m = new all.default.Manager(d);
        });

//# sourceMappingURL=main.js.map

@mhegazy
Copy link
Contributor

mhegazy commented Jan 12, 2016

looks like a duplicate of #4434

@mhegazy mhegazy closed this as completed Jan 12, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Jan 12, 2016
@SetTrend
Copy link
Author

No, not at all. As you can see from #4434, the suggestion there still suggests to create a number of define() statements - or System.register().

@SetTrend
Copy link
Author

Please re-open.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 12, 2016

not really. see my comment in #4434 (comment).

@SetTrend
Copy link
Author

Ah, OK. I was only looking at the conclusion at the end of the thread. Fully agree.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

2 participants