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

Allow to specify includeFile position. #4419

Merged
merged 2 commits into from Jul 20, 2015

Conversation

Projects
None yet
2 participants
@andyli
Member

andyli commented Jul 17, 2015

Currently haxe.macro.Compiler.includeFile() includes the input file into the call site, where is usually inside __init__. It is problematic:

Problem 1. unable to include strict-mode incompatible file

Haxe by default generate JS output in strict mode. But the input file (e.g. jQuery!) may not be strict mode compatible, thus there will be run-time error being thrown.

Promlem 2. unable to extend classes from the included file

Since the haxe class members are defined before __init__s, we cannot extend an external class. Example:

// external.js
var External = function() { };
extern class External {
    static function __init__():Void {
        haxe.macro.Compiler.includeFile("external.js");
    }
}

class Test extends External {
    static function main():Void {
        trace("ok");
    }
}

JS output:

// Generated by Haxe
(function (console) { "use strict";
function $extend(from, fields) {
    function Inherit() {} Inherit.prototype = from; var proto = new Inherit();
    for (var name in fields) proto[name] = fields[name];
    if( fields.toString !== Object.prototype.toString ) proto.toString = fields.toString;
    return proto;
}
var Test = function() { };
Test.main = function() {
    console.log("ok");
};
Test.__super__ = External;
Test.prototype = $extend(External.prototype,{
});
// external.js
var External = function() { };
Test.main();
})(typeof console != "undefined" ? console : {log:function(){}});

run-time error:

/Users/andy/Documents/workspace/TestHaxe/Test.js:14
Test.prototype = $extend(External.prototype,{
                                 ^
TypeError: Cannot read property 'prototype' of undefined
    at console.undefined.log (/Users/andy/Documents/workspace/TestHaxe/Test.js:14:34)
    at Object.<anonymous> (/Users/andy/Documents/workspace/TestHaxe/Test.js:19:3)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:929:3
Error: Command failed with error 8

This PR

This PR adds an argument position to includeFile. It is defaulted to Top, which is the top of the output, outside our top-level closure. The JS output of the above example will become:

// Generated by Haxe
// external.js
var External = function() { };
(function (console) { "use strict";
function $extend(from, fields) {
    function Inherit() {} Inherit.prototype = from; var proto = new Inherit();
    for (var name in fields) proto[name] = fields[name];
    if( fields.toString !== Object.prototype.toString ) proto.toString = fields.toString;
    return proto;
}
var Test = function() { };
Test.main = function() {
    console.log("ok");
};
Test.__super__ = External;
Test.prototype = $extend(External.prototype,{
});
Test.main();
})(typeof console != "undefined" ? console : {log:function(){}});

andyli added a commit to andyli/haxe that referenced this pull request Jul 18, 2015

@andyli andyli force-pushed the andyli:includeFile branch from fb1d293 to 523d5a1 Jul 18, 2015

andyli added a commit that referenced this pull request Jul 20, 2015

Merge pull request #4419 from andyli/includeFile
Allow to specify includeFile position.

@andyli andyli merged commit 183928c into HaxeFoundation:development Jul 20, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dionjwa

This comment has been minimized.

Contributor

dionjwa commented Jul 28, 2015

+1 This will be really useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment