TypeScript compiler regression

[malept]

The commit ffbd700 introduced a regression. The following repro steps fail with 5.3.0 and 5.3.1 and succeeds with 5.2.5.

Repro steps

1. Install Electron Forge
2. Run electron-forge init something
3. Run cd something
4. Run electron-forge package

Expected

A package is created for the host platform & arch.

Actual

✔ Checking your system
✔ Preparing to Package Application for arch: x64
⠋ Compiling ApplicationCouldn't set up compilers: Cannot find module 'typescript/package.json'
✖ Compiling Application

An unhandled error has occurred inside Forge:
Cannot find module 'typescript/package.json'
Error: Cannot find module 'typescript/package.json'
    at Function.Module._resolveFilename (module.js:469:15)
    at Function.Module._load (module.js:417:25)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at TypeScriptCompiler.getCompilerVersion (/tmp/something/node_modules/electron-compilers/lib/js/typescript.js:70:12)
    at Object.keys.reduce (/tmp/something/node_modules/electron-compile/lib/compiler-host.js:496:35)
    at Array.reduce (native)
    at CompilerHost.saveConfigurationSync (/tmp/something/node_modules/electron-compile/lib/compiler-host.js:488:72)
    at createCompilerHostFromConfiguration (/tmp/something/node_modules/electron-compile/lib/config-parser.js:323:7)
    at /tmp/something/node_modules/electron-compile/lib/config-parser.js:80:12
    at next (native)
    at step (/tmp/something/node_modules/electron-compile/lib/config-parser.js:179:191)
    at /tmp/something/node_modules/electron-compile/lib/config-parser.js:179:361

When npm install is run, this is one of the warnings (electron-compilers@5.3.1 is installed):

UNMET PEER DEPENDENCY typescript@>=1.6

[MarshallOfSound]

/cc @kwonoj @paulcbetts typescript shouldn't be a required peer-dep as lots of people use electron-compile without using typescript.

/cc @malept Can we pin our electron-compile version until this is fixed

[kwonoj]

@MarshallOfSound so previously, typescript was dependency regardless of someone uses TS or not (https://github.com/teppeis/typescript-simple/blob/master/package.json#L28), this regression comes by now compiler correctly requires compiler optional, just missing correct handling if compiler is really missing.

Compiler behavior should be fixed to work without installation of TS but I still think typescript installation need to be optional instead of requiring it in behind of scene. maybe optionalDep would be more sense than peerDep though.

[MarshallOfSound]

@kwonoj Fair enough, however I think that the change that was made should have been considered breaking. Anyone currently using typescript will find themselves unable to do so with just a minor bump in this package. Is that correct?

[kwonoj]

@MarshallOfSound for versioning perspective, I do agree. actually, when I proposed PR (#50) I proposed it with breaking changes.

[kwonoj]

@MarshallOfSound please review #53, I've added handler also made TS compiler as direct dependency to avoid regression in minor version bump.

I'll create separate PR for making TS compiler as peerDep for major version bump.

[anaisbetts]

Sorry all, I didn't remember that we'd check the version, totally agree that this should be fixed asap

[malept]

Fixed, thanks Paul.