bundling broken since 4.0.6

[tamird]

https://github.com/frankwallis/plugin-typescript/commit/58cc11c5d0cebf91ba8d00e881fbde1e1b4c948a seems to be the culprit. Manually reverting that commit locally fixed bundling for us in https://github.com/cockroachdb/cockroach/tree/master/ui/next. Without reverting it, we see:

$(npm bin)/jspm bundle app
     Building the bundle tree for app...
TypeScript transpiling to CommonJS, consider setting module: "system" in typescriptOptions to transpile directly to System.register format
TypeScript file:///Users/tamird/src/go/src/github.com/cockroachdb/cockroach/ui/next/app/containers/cluster.tsx:1:24
TypeScript Cannot find module 'react'. (TS2307)
TypeScript file:///Users/tamird/src/go/src/github.com/cockroachdb/cockroach/ui/next/app/containers/cluster.tsx:8:12
TypeScript JSX element implicitly has type 'any' because the global type 'JSX.Element' does not exist. (TS2602)
TypeScript file:///Users/tamird/src/go/src/github.com/cockroachdb/cockroach/ui/next/app/containers/cluster.tsx:8:12
TypeScript JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists (TS7026)
....

Unfortunately, there is a change in 4.0.9 (#123) which we need, so we're at an impasse. It is also impossible to revert this on master as it no longer reverts cleanly.

[frankwallis]

The issue here is that containers/cluster.tsx is not referencing typings/browser/main.d.ts so it doesn't know about the react module.

The simplest solution would be to include the typings files using tsconfig.json instead of /// <references:

{
  "compilerOptions": {
    "target": "ES5",
    "module": "commonjs",
    "jsx": "react",
    "noImplicitAny": true,
    "noEmit": true
  },
  "files": [
    "../typings/main.d.ts",
    "./interfaces/protos.d.ts",
    "../lib/protos/protos.d.ts",
    "./app.tsx"
  ]
}

It seems that plugin-typescript is not following the same algorithm as tsc when resolving dependencies which is described in this issue Microsoft/TypeScript#1066. I think I can probably improve this, so I will investigate.

I will also look into how it was working in 4.0.5 as this seems like a bug although it may have been due to the files being loaded in a different order.

[frankwallis]

I'm not going to change the resolution algorithm of plugin-typescript as I think it will have a negative impact on performance when compiling in the browser. In order to match up with tsc I think I would have to load the entire program before type-checking anything. I also think it could lead to the dependencies being hard to reason about (you remove an import statement and the build fails because the imported file had a /// <reference tag which was allowing another unit to compile).

Being able to type-check files in the browser means that plugin-typescript has to be more restrictive here. My recommendation is to avoid /// <reference tags and use tsconfig.json instead.

[tamird]

This is a bummer - using files is pretty inconvenient in the absence of filesGlob support :(

[aluanhaddad]

My recommendation is to avoid /// <reference tags and use tsconfig.json instead.

I can't second this enough. Use of /// <reference tags, especially when combined with use of external modules, will lead to code that is hard to maintain and tough to reason about. They are rarely necessary and should be kept to an absolute minimum.

[frankwallis]

@tamird you shouldn't need to list all the files in the build, just the declaration files and the entry point (as above). Maybe that helps?

[tamird]

@frankwallis sorry for the long delay here. Yes, just the declaration file and entry point works:

...
  "files": [
    "typings/index.d.ts",
    "app/app.tsx"
  ]
...

However, this also has the side effect of causing editors like vscode to ignore test files entirely (because they are not reachable from the entry point). Is there a workaround you can suggest?