theintern / intern-examples

A collection of examples for the Intern testing library
theintern.io
85 stars 83 forks source link

TypeScript example compilation errors #52

Open draperd opened 6 years ago

draperd commented 6 years ago

I'm trying to write functional tests using TypeScript and have followed the example provided. However I'm getting compilation errors:

test/functional/example.ts(1,9): error TS4023: Exported variable 'registerSuite' has or is using name 'ObjectSuiteDescriptor' from external module "/home/dave/Git/account-management/node_modules/intern/lib/interfaces/object" but cannot be named.
test/functional/example.ts(1,9): error TS4023: Exported variable 'registerSuite' has or is using name 'ObjectSuiteFactory' from external module "/home/dave/Git/account-management/node_modules/intern/lib/interfaces/object" but cannot be named.
test/functional/example.ts(1,9): error TS4023: Exported variable 'registerSuite' has or is using name 'Tests' from external module "/home/dave/Git/account-management/node_modules/intern/lib/interfaces/object" but cannot be named.

...generated from the line:

const { registerSuite } = intern.getInterface('object');

...which is taken directly from the example

The tests themselves do actually run when treated as JavaScript (however I needed to modify the leadfoot/keys import to be: const keys = require('@theintern/leadfoot/keys').default;)

draperd commented 6 years ago

This can be resolved by importing:

import intern from 'intern';

as the first line, however I'm not sure whether or not that is an acceptable solution? If so, I can create a PR.

kitsonk commented 6 years ago

I assume in your tsconfig.json (or when you are compiling with TSC) you have "declaration": true. If this is the case, TypeScript is generating a .d.ts for each of your TypeScript files, including the tests. Typically you only do this when you are going to redistribute your code as JavaScript and need the declaration files to go along with it.

You are then exporting something from test/functional/example.ts which then when TypeScript is generating the file, it cannot create a valid .d.ts without you being explicit where certain type definitions are coming from. This is why doing the import resolves the issue, because TypeScript can name those types to generate a valid definition file.

There are three solutions to this:

draperd commented 6 years ago

Thanks for the quick reply...

Yes, this project in question is exporting modules for import into other projects so you're quite correct that we have "declaration": true in our tsconfig.json file... this is in the main tsconfig.json file at the root of the project rather than a nested tsconfig.json at the root of the tests folder.

I'm happy with my resolution to the problem but wanted to flag up the issue in case other developers hit the same problem.

jason0x43 commented 6 years ago

Note that the example uses a separate tsconfig.json for the tests. That config includes the following types directive:

"types": ["intern"]

This has the same effect as import intern from 'intern';, which will load the global intern variable type.

draperd commented 6 years ago

I had a separate tsconfig.json file that had that "types": ["intern"] entry in it. It didn't prevent me seeing the error appearing in either the VSCode console or the output from running TSC.

I'm just reporting what I experienced in case this is something that other people encounter.

Maybe I needed to run TSC from within the tests folder directly - however, as with a lot of TypeScript errors this isn't immediately obvious as to the cause of the problem.

jason0x43 commented 6 years ago

Could you post a sample project somewhere? The example itself builds properly whether or not declarations are enabled in "tests/tsconfig.json", and I'd like to look into your specific situation in a bit more detail.

In general, if you're creating a project that's exporting test functionality (like helper functions for Intern), it would make sense to have a single tsconfig.json file and include the "types": ["intern"] directive in that file (or import 'intern'; somewhere). The reason the example uses a separate tsconfig for tests is because for a typical project the test code isn't part of the application being tested. We don't want to create any accidental dependencies between the application and Intern, so the application's tsconfig doesn't load intern types.

draperd commented 6 years ago

Unfortunately I can't post my project as-is (without disclosing private company information) but could look to create a sample project with similar configuration when I get a chance.

I agree that it makes sense to keep the tsconfig separate, but the example is extending the configuration - it might be better to define a separate config without explicitly without extending.

My point here isn't to criticize for the sake of it... I really like Intern, I just want to help reduce barriers to entry which was I raised the issue. It's not especially blocking me at the moment, I just wanted to make you aware it had happened.

Our main tsconfig looks like this though:

{
    "compilerOptions": {
        "allowJs": false,
        "allowSyntheticDefaultImports": true,
        "declaration": true,
        "jsx": "react",
        "module": "commonjs",
        "noImplicitAny": false,
        "outDir": "dist",
        "removeComments": true,
        "inlineSourceMap": true,
        "strictNullChecks": true,
        "lib": [
            "es6",
            "dom"
        ],
        "target": "es5"
    },
    "include": [
        "node_modules/@types/**/*.d.ts",
        "src/**/*.ts",
        "src/**/*.tsx"
    ],
    "exclude": [
        "deps**/*"
    ]
}

...and the nested version looks like this:

{
  "declaration": false,
  "extends": "../tsconfig.json",
  "compilerOptions": {
      "baseUrl": "..",
      "types": [ "intern" ]
  },
  "include": [
      "./**/*.ts"
  ]
}
jason0x43 commented 6 years ago

I definitely appreciate the information. This just seems more like an issue with a specific use case than a problem with the example.

There wouldn't really be any benefit to separating the configs here. I mean, the tests should be built with almost all the same settings as the application, with only a few test-specifi differences, which is the intended use case for extends.

Note that "declaration" is a compiler option, so should be in compilerOptions in the nested config. While having it outside compilerOptions won't cause a build error, it has no effect there. That could be causing problems, since the nested config isn't disabling the declaration statement as intended.