Broken Date type reflection mechanizm

[MichalLytek]

There is an issue in a transpilation mechanizm that ts-node also use:
TypeStrong/ts-node#511

import "reflect-metadata";

const CollectMetadata: PropertyDecorator = (target, propertyKey) => {
    const ReflectType = Reflect.getMetadata("design:type", target, propertyKey);
    console.log(ReflectType);
    console.log(ReflectType === Date);
}

class Test {
    @CollectMetadata
    property: Date;
}

The workaround is to use --type-check flag with ts-node.
Is there an equivalent of that option in ts-jest?
Thanks!

[GeeWee]

Interesting. Not currently. I'm not sure what the --type-check flag actually does.

[akheron]

I also have this problem. I made a repo that demonstrates the issue: https://github.com/akheron/ts-jest-test

[kulshekhar]

I'm not sure about this but I suspect this would involve walking through the AST

[jelling]

I ask that we reopen this issue. Both this issue and #445, which I created, were closed but we have 4 people having the same issue in the span of a few days so this issue is probably not going away.

As mentioned in #445, a similar issue is solved in ts-node by using the --type-check flag. I did a brief dive into their code, and while I'm not an expert in ts compilation, it seems like they're just setting additional compiler settings.

[GeeWee]

I definitely agree that this shouldn't be closed, (but to me it seems open). Would love a PR (or a test case on a ts-jest branch) if anyone feels up for the task.

[kulshekhar]

@jelling

it seems like they're just setting additional compiler settings.

I couldn't find info on this but if you can tell me which compiler settings need to be set, this can be added to ts-jest. If it's something more than compiler settings, I'm going to need help here

[MichalLytek]

I definitely agree that this shouldn't be closed, (but to me it seems open). Would love a PR (or a test case on a ts-jest branch) if anyone feels up for the task.

@GeeWee I've created a PR with test suite to show the problem 😉

[GeeWee]

That's wonderful thank you so much! Hopefully we can get this looked at soon!

(if anyone else reading wants to give it a shot, you're also very welcome, we're not a huge base of contributors and we all have stuff to do)

[lstkz]

I am having the same problem. Here is a quick workaround.

const forceType = (object: any) => (
  target: any,
  propertyKey: string,
) => {
  Reflect.defineMetadata('design:type', object, target, propertyKey);
};


class MyType {
    @minDate('now')
    @optional()
    @forceType(Date) // add extra annotation here
    date?: Date;
  }

Also, there are similar problems with arrays.

type StringOrNumber = string | number;

class MyType {
  a: string[], // OK, array is emitted
  b: Array<string | number>, // Wrong, object is emited
  c: StringOrNumber[], // OK
  d: MyCustomClass[], // OK
}

[GeeWee]

I've taken a look at ts-node.

It seems that if typechecking is set, they create a typescript language service and make that emit the output. Details here

Relevant parts with some stuff commented out is below:

if (typeCheck) {
    // Set the file contents into cache.
    const updateMemoryCache = function (code: string, fileName: string) {
      //...
      }
    }

    // Create the compiler host for type checking.
    const serviceHost = {
      getScriptFileNames: () => Object.keys(memoryCache.versions),
      getScriptVersion: (fileName: string) => {
           //..
      },
      getScriptSnapshot (fileName: string) {
          //...
      },
      fileExists: debugFn('fileExists', fileExists),
      readFile: debugFn('getFile', readFile),
      readDirectory: debugFn('readDirectory', ts.sys.readDirectory),
      getDirectories: debugFn('getDirectories', ts.sys.getDirectories),
      directoryExists: debugFn('directoryExists', ts.sys.directoryExists),
      getNewLine: () => EOL,
      getCurrentDirectory: () => cwd,
      getCompilationSettings: () => config.options,
      getDefaultLibFileName: () => ts.getDefaultLibFilePath(config.options),
      getCustomTransformers: () => transformers
    }

    const service = ts.createLanguageService(serviceHost)

    getOutput = function (code: string, fileName: string, lineOffset: number = 0) {
      // Must set memory cache before attempting to read file.
      updateMemoryCache(code, fileName)

      const output = service.getEmitOutput(fileName)

      // Get the relevant diagnostics - this is 3x faster than `getPreEmitDiagnostics`.
      const diagnostics = service.getCompilerOptionsDiagnostics()
        .concat(service.getSyntacticDiagnostics(fileName))
        .concat(service.getSemanticDiagnostics(fileName))

      const diagnosticList = filterDiagnostics(diagnostics, ignoreDiagnostics)

      if (diagnosticList.length) {
        throw new TSError(formatDiagnostics(diagnosticList, cwd, ts, lineOffset))
      }

      if (output.emitSkipped) {
        throw new TypeError(`${relative(cwd, fileName)}: Emit skipped`)
      }

      // Throw an error when requiring `.d.ts` files.
      if (output.outputFiles.length === 0) {
        throw new TypeError(
          'Unable to require `.d.ts` file.\n' +
          'This is usually the result of a faulty configuration or import. ' +
          'Make sure there is a `.js`, `.json` or another executable extension and ' +
          'loader (attached before `ts-node`) available alongside ' +
          `\`${basename(fileName)}\`.`
        )
      }

      return [output.outputFiles[1].text, output.outputFiles[0].text]
    }

[phlmn]

I am also experiencing this issue when using typeorm (like #452). Is there any workaround yet?

[stevenmusumeche]

Same issue here with TypeORM.

[GeeWee]

Hi.

We've added a new experimental flag, that might solve these issues:
It's useExperimentalLanguageServer and it's set in the ts-jest key, just like the rest of the settings.

Please report back whether:
a) This fixes the issues
b) What kind of performance-hit you're taking.

More context in PR #505

[jonathanrdelgado]

Hey @GeeWee, first off, thanks for your work on this, that PR looks like it took a while. I implemented the flag this morning.

• I received a ton of errors like TSC language server encountered errors while transpiling. Errors: [object Object],[object Object],[object Object]
• Editing ts-jest/dist/transpiler.js:59:15 to have d.messageText.messageText fixed the output so I could actually read the error.
• The errors were Errors: Cannot write file '/app/src/file.js' because it would overwrite input file.
• Building this project outside of ts-jest works normally, here is our TSConfig:

{
    "compilerOptions": {
        "target": "es2017",
        "module": "commonjs",
        "allowJs": true,
        "outDir": "./dist",
        "strict": true,
        "inlineSourceMap": true,
        "experimentalDecorators": true,
        "emitDecoratorMetadata": true,
        "typeRoots": ["node_modules/@types", "./src/typedefs"]
    },
    "include": ["src/**/*"],
    "exclude": ["node_modules", "src/**/*.test.ts", "src/**/__mocks__/*"]
}

It looks like the new service is trying to overwrite source files on disk, instead of using the proper outDir.

I'll continue debugging on my side, just wanted to give you some feedback on the flag. Not sure if it fixes the TypeORM issues yet.

[kulshekhar]

@jonathandelgado ts-jest deletes the outDir config (see the accompanying comment for the reason)

Would it be possible for you to comment out the equivalent line in your node_modules/ts-jest/dist directory and see if that

• fixes the issue you've reported AND
• addresses the TypeORM/related issue

?

[GeeWee]

Thanks a lot for the feedback, good thing we didn't roll this out properly yet :)

[jonathanrdelgado]

@kulshekhar Good call on the outDir deletion. I went ahead and commented that out locally and was able to get around that issue. However, as noted in that comment, the sourcemaps are incorrect as expected.

Immediately following fixing that, I started getting a ton of type errors that I wouldn't receive in a normal build (even including tests in that build). One that specifically stood out was (21,29): Property 'user' does not exist on type 'Request'. For context, we use express and manually inject a user attribute in middleware. However, we have a namespace override in our ./src/typedefs folder, which is in the above config under typeRoots.

I did a preliminary search in this repo for any "gotchas" that would be similar to the outDir change, but I didn't see anything. I assume that property just isn't being respected by the new language service? I took a look at the PR, but to be honest, I don't know enough about the Typescript compiler to offer any more meaningful feedback past that.

I'll try to hop back on this tonight and continue debugging, thank you both for your help on this!

[GeeWee]

Did you have any other type-errors that were "direct" imports? e.g. not related to any namespace overrides / declaration merging?

[GeeWee]

We've updated some of the ts language server code. Try pulling the latest version - I don't think it'll fix the issues but it's worth a shot.

Otherwise, It'd be great if we could get an isolated test case more that demonstrates this behaviour.

[jonathanrdelgado]

Thanks for pinging me on this, I've been working on some other projects recently, so this fell a little bit down my backlog. I'll pull down the latest changes and if I still have the problem I'll get an isolated repo setup for you within the next few days. Thanks again!

[csuich2]

@GeeWee It looks like @akheron put together a repo with an isolated test case here: https://github.com/akheron/ts-jest-test

[jonathanrdelgado]

Awesome, thanks for posting that @csuich2, you just saved me a half an hour!

[jonathanrdelgado]

To follow up, is appears the repo @csuich2 linked works when you implement the experimental flag, so that's awesome.

I went ahead and grabbed the most recent changes from ts-jest, commented out the build line, fixed the d.messageText.messageText thing and I'm still having issues related to the new service. I'm getting errors that state TSC language server encountered errors while transpiling. Errors: (That's not a typo, there are no errors after that point). I spent some time looking into them, but didn't find anything really promising.

I think this should be moved to another ticket, though, since it seems unrelated to the current issue.

[GeeWee]

(Interestingly enough, the typescript team has told me on twitter that the reflection might be a bug in their implementation of transpileModule, hold your horses for this one)

[GeeWee]

The bug should be fixed on the typescript teams end: See microsoft/TypeScript#16872
Can anyone confirm this works with typescript@next? (without using the experimentalLanguageServer flag)

[csuich2]

Just confirmed the issue is fixed with typescript@next. I'll confirm it is fixed in typescript@2.9.1 when it is released later this week.

Thanks!

[GeeWee]

Awesome. Closing.

[csuich2]

For the record, I've confirmed this is working with TS 2.9.1 as well.