Issue with TypeScript 2.9.1

[manu-st]

Expected Behaviour

Code should include a require to run properly.

Actual Behaviour

require is missing in generated code

Steps to Reproduce the Problem

Not sure.

Location of a Minimal Repository that Demonstrates the Issue.

I have a file and when I'm compiling using TypeScript 2.9 I get the following:

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
Object.defineProperty(exports, "__esModule", { value: true });
const graph_1 = __webpack_require__(50);
class CollectionGraphStore extends graph_1.GraphStore {
    constructor() {
        super(new ProcedureGraphOptions_1.ProcedureGraphOptions());
....

As you can see ProcedureGraphOptions_1 is not included.

If I generate the same code using TypeScript 2.8.3, then I get:

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
Object.defineProperty(exports, "__esModule", { value: true });
const graph_1 = __webpack_require__(51);
const ProcedureGraphOptions_1 = __webpack_require__(122);
class CollectionGraphStore extends graph_1.GraphStore {
    constructor() {
        super(new ProcedureGraphOptions_1.ProcedureGraphOptions());
....

Unfortunately I'm at loss at why it is not generating the missing require. If I use tsc 2.9.1 from the command line to generate the JavaScript code, the code is properly generated:

"use strict";
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
Object.defineProperty(exports, "__esModule", { value: true });
const graph_1 = require("Controls/graph");
const ProcedureGraphOptions_1 = require("Experiment/procedures/ProcedureGraphOptions");
class CollectionGraphStore extends graph_1.GraphStore {
    constructor() {
        super(new ProcedureGraphOptions_1.ProcedureGraphOptions());
...

Any idea?

[johnnyreilly]

None at all I'm afraid; that seems super strange. What does the source code look like that generates this code? There's nothing changed in ts-loader that would seem relevant.

[johnnyreilly]

It feels like this might be an issue with TypeScript rather than ts-loader - the code is generated by TSC and we just use what it pumps out. Could be worth sticking a console.log in the ts-loader codebase at the point where TypeScript emits the code and seeing if it's there at all with 2.9.1: https://github.com/TypeStrong/ts-loader/blob/master/src/index.ts#L71

That would be useful to know. Just hack it into the version in your node_modules and see what you find. If it's missing there it's probably a TypeScript issue

[manu-st]

I'll have a look at it and report back.

[arackaf]

I'm seeing a similar issue in our code base - an import is being dropped which is leading to a runtime error.

This does indeed seem to be an issue with ts-loader. Running tsc alone, I'm seeing correct code emitted. Moreover, when I rename the file to be .js, the error goes away, so it doesn't appear to be a webpack tree-shaking but, either. For me, here's the offending code, in part

import { AgGridStateManager } from "app/util/agGrid/helpers";
import router from "app/util/router";
import user from "app/util/user";

const userIsClient = user.isClient();

let legacyColumnMapping = {
  "col-select-type": "type",
  "col-select-id": "id",
  "col-select-last-login": "lastLogin"
};

export default class ContactsAgGridStateManager extends AgGridStateManager {
  constructor() {
    super({
      defaultColumns: [
        // right boolean is whether to HIDE initially or not
        ["select", false],
        ["type", userIsClient],
        ["primaryInsuranceVerificationStatusCode", true],
        ["lastLogin", true]
      ],
      gridSettingsName: "contacts-grid-settings",
      storage: router.currentModuleStorage
    });
  }
}

The import for import router from "app/util/router"; is dropped completely. But tsc alone does indeed seem to respect it, and emit it.

Also, as others have noted, reverting back to TS 2.8.3 also fixes things.

---

Also, adding something silly, like

const x = router.x;

to the very top level of this module, fixes the problem, so it definitely seems as though TS is wrongly tree shaking a used import out of the file.

[arackaf]

Update: the same bug appears with awesome-typescript-loader, so there does seem to be something about ts 2.9.1 that's causing webpack loaders to emit incorrect code.

cc @DanielRosenwasser

[arackaf]

I'm working on putting together a workaround, whereby I just tell ts-loader to run an older, working version of typescript (assuming you have the transpileOnly option already set, and are doing type checking in a different process, which I already am). I think the instance config option might be for that, but the docs are non-existent for it.

I tried putting instance: "folder" where folder is a directory with a different version of typescript installed in node_modules, but that appears to do nothing. The closest to docs I could find for it were from this issue: #647

Hopefully that might help someone else.

[manu-st]

@arackaf Using an old version of the TypeScript compiler might not be adequate if you want to use the latest language features. We just need to figure out why some require are missing.

[arackaf]

@manu-st - no disagreement. My point was based on the premise that you were already NOT doing type checking with the webpack plugin. We gave up on that months ago - the false positive errors were impossible to figure out, so we just switched on transpileOnly, and we have a separate tsc task to do our type checking.

If I could figure out how to get ts-loader to run ts 2.8 in transpileOnly mode, with tsc running normally on 2.9.1, I'd be all set, at least as a temporary workaround, until this is fixed.

[johnnyreilly]

@arackaf thanks for sharing your code - that's helpful. Also it's interesting to know that awesome-typescript-loader is experiencing the same issue. This makes me think more than ever that there's an issue with the 2.9.1 API.

Would one of you be able to give this a try please:

Stick a console.log in the ts-loader codebase at the point where TypeScript emits the code and seeing if it's there at all with 2.9.1: https://github.com/TypeStrong/ts-loader/blob/master/src/index.ts#L71

That would be useful to know and should confirm the theory (or not). No need to compile a new version of ts-loader; just hack the console.log into the compiled JS version in your node_modules and see what you find. If the require is missing there it's probably a TypeScript API issue.

Assuming this is a TypeScript API issue it would be good to raise an issue with the TypeScript project to let them know. (I don't know if @DanielRosenwasser will necessarily see the comment here) There was another issue with 2.9.1 which I reported which has already been fixed (microsoft/TypeScript#24625); the team are super responsive! If one of you could do that I'd greatly appreciate it.

[johnnyreilly]

the false positive errors were impossible to figure out

As a complete aside @arackaf (and I don't want to derail the issue) this made me curious. Would be interested to know what the false positives were. I'm still happily using the separate checker. Obviously your mileage may vary etc but I am interested in issues.

[arackaf]

@johnnyreilly Ok, I did what you said, and sure enough, the import (it's an ES6 import, not a require()) is indeed gone for the file in question, but only with transpileOnly set to true. Once I remove that, then the emitted source is correct, and has the import in it again.

I guess this dovetails nicely with the false positives you were asking about. Basically, ts-loader complains about a ton of ts errors, usually involving typings and libs. It ignores some legacy declare var statements, and then errors out on things like global $ or ko references, and it complains about things like addEventListener, location, open, etc existing on window, etc. tsc reports none of these errors, so we use ts-loader with transpileOnly, and then have a second tsc watch for error reporting

While I have you, can I trouble you for some help in getting the instance property working? Forcing ts-loader to use 2.8.3 while this is worked out is probably my best bet, but I can't for the life of me figure out how to configure it.

Needless to say, if there's anything at all I can do to help get this fixed, please don't hesitate. /cc @DanielRosenwasser

[manu-st]

I confirm that I'm also using ts-loader with transpileOnly and we use ForkTsCheckerWebpackPlugin to perform the type checking as we found this to be faster.

[manu-st]

I also confirm that dropping transpileOnly is causing the code to be generated properly. However this is not a solution for us since it takes 120s with this option disabled and around 45s when it is set to true.

[arackaf]

The most expeditious band-aid / workaround would be to get that instance config property working. If we're only using ts-loader for transpileOnly then we could probably pipe it through a side install of ts 2.8 and still get the same output. (while using our type checker as usual)

Does anyone have any info on how to do that???

[johnnyreilly]

@arackaf

Needless to say, if there's anything at all I can do to help get this fixed, please don't hesitate.

Thanks for investigating that's awesome. It sounds like there is a bug with the transpileModule API in TypeScript 2.9.1. Please could you raise a ticket against the Typescript project and link back to this issue?