acornjs / acorn

A small, fast, JavaScript-based JavaScript parser
10.59k stars 883 forks source link

Duplicate exports for typescript enum type #513

Closed Abrissirba closed 7 years ago

Abrissirba commented 7 years ago

I'm writing an angular 2 appliation using the angular-cli which use webpack which in turn use acorn. The application is written in typescript. After upgrading angular-cli to the latest version I encountered an error with duplicate errors for my enum types, see example below.

export enum ReportFrequencies
{
    Daily = 6413,
    Weekly = 5593,
    Monthly = 5592,
    BiMonthly = 6452,
    Quarterly = 6412,
    HalfYearly = 6644
}

export namespace ReportFrequencies {
    function isIndex(key): boolean {
        let n = Number(key);
        return String(n) === key && n >= 0;
    }

    export function GetDescription(reportFrequencies: ReportFrequencies) {
        return ReportFrequencies[reportFrequencies].match(/[A-Z][a-z]+/g).join(' ');
    }
}

I traced down the error and saw that it was introduced when webpack started using version 4.x.x of acorn which checks for duplicate exports.

The code example seems like valid typescript and according to https://www.typescriptlang.org/play/ and https://www.typescriptlang.org/docs/handbook/declaration-merging.html. However the checkExport method, https://github.com/ternjs/acorn/blob/01c6ad27d42ff222434ecf9ba3c21fbce2bc3ef8/src/statement.js#L624, throws an error for this code.

Original issue in angular-cli repo: https://github.com/angular/angular-cli/issues/4735

RReverser commented 7 years ago

I guess you wanted to report this to another project? Acorn doesn't support parsing TypeScript, only JavaScript code (unless you had a problem with some specific plugin that allows that for you).

Abrissirba commented 7 years ago

Ok, the typescript code is code is transpiled down to the following code:

define(["require", "exports"], function (require, exports) {
    "use strict";
    var ReportFrequencies;
    (function (ReportFrequencies) {
        ReportFrequencies[ReportFrequencies["Daily"] = 6413] = "Daily";
        ReportFrequencies[ReportFrequencies["Weekly"] = 5593] = "Weekly";
        ReportFrequencies[ReportFrequencies["Monthly"] = 5592] = "Monthly";
        ReportFrequencies[ReportFrequencies["BiMonthly"] = 6452] = "BiMonthly";
        ReportFrequencies[ReportFrequencies["Quarterly"] = 6412] = "Quarterly";
        ReportFrequencies[ReportFrequencies["HalfYearly"] = 6644] = "HalfYearly";
    })(ReportFrequencies = exports.ReportFrequencies || (exports.ReportFrequencies = {}));
    (function (ReportFrequencies) {
        function isIndex(key) {
            var n = Number(key);
            return String(n) === key && n >= 0;
        }
        function GetDescription(reportFrequencies) {
            return ReportFrequencies[reportFrequencies].match(/[A-Z][a-z]+/g).join(' ');
        }
        ReportFrequencies.GetDescription = GetDescription;
    })(ReportFrequencies = exports.ReportFrequencies || (exports.ReportFrequencies = {}));
});

Is this not valid in acorn?

marijnh commented 7 years ago

There are no language-level exports in that code, so I can't see how Acorn could raise an error about those for that code. (I.e. checkExports simply won't run when parsing it.)

Abrissirba commented 7 years ago

Ok, thanks for the help but I don't have enough knowledge about the webpack echosystem to be able to know why the code breaks after upgrading webpack and acorn versions. Hopefully I can get some help in the issue at the angular-cli repo.

Abrissirba commented 7 years ago

I have been digging a bit more now and looked at the javascript code that typescript transpiles down to and that acorn then tries to parse. As you can see there are two exports there and I guess that is not valid according to some spec? Is it possible to turn the duplicate errors check off?

export var ReportFrequencies;
(function (ReportFrequencies) {
   ReportFrequencies[ReportFrequencies["Daily"] = 6413] = "Daily";
   ReportFrequencies[ReportFrequencies["Weekly"] = 5593] = "Weekly";
   ReportFrequencies[ReportFrequencies["Monthly"] = 5592] = "Monthly";
   ReportFrequencies[ReportFrequencies["BiMonthly"] = 6452] = "BiMonthly";
   ReportFrequencies[ReportFrequencies["Quarterly"] = 6412] = "Quarterly";
   ReportFrequencies[ReportFrequencies["HalfYearly"] = 6644] = "HalfYearly";
})(ReportFrequencies || (ReportFrequencies = {}));

export var ReportFrequencies;
(function (ReportFrequencies) {
   function isIndex(key) {
       var n = Number(key);
       return String(n) === key && n >= 0;
   }
   function GetDescription(reportFrequencies) {
       return ReportFrequencies[reportFrequencies].match(/[A-Z][a-z]+/g).join(' ');
   }
   ReportFrequencies.GetDescription = GetDescription;
})(ReportFrequencies || (ReportFrequencies = {}));
marijnh commented 7 years ago

As you can see there are two exports there and I guess that is not valid according to some spec?

Yes, the spec explicitly forbids exporting the same name twice. I consider this a TypeScript bug and I'm not interested in adding a workaround option to Acorn.