Merge in compilerOptions prior to calling parseJsonConfigFileContent

[ianschmitz]

This PR fixes a deficiency in #173 (comment), as we were merging the compilerOptions in too late in the pipeline, which meant that if someone wanted to override a configuration option that was a string value example (such as module or target), they would have to use the typescript enum integer value instead (module: 1 instead of module: "es5").

Here is an example where I'm overriding in the webpack config of create-react-app. My tsconfig.json specifies "moduleResolution": "node", but i override it to "classic" here:

/cc @Timer

[ianschmitz]

Note that previously we were merging in isolatedModules: "false" into the tsconfig, but from as best as i can tell that had no effect as it wasn't being merged in to the config.compilerOptions.

To prevent a breaking change I simply removed this logic

[johnnyreilly]

To prevent a breaking change I simply removed this logic

I've just been looking back on the history; this method used to look something like this:

static loadProgramConfig(configFile: string) {
    return ts.parseJsonConfigFileContent(
      // Regardless of the setting in the tsconfig.json we want isolatedModules to be false
      Object.assign(ts.readConfigFile(configFile, ts.sys.readFile).config, { isolatedModules: false }),
      ts.sys,
      path.dirname(configFile)
    );
  }

I believe that this compiler flag is significant ; @piotr-oles can you advise?

I'm uncomfortable with this coming out to be honest.

Still evidence is king. It would be good to take a slightly older version of fork-ts-checker-webpack-plugin, hack in a console.log to the loadProgramConfig to determine whether isolatedModules: "false" used to be set and we were just mistaken. Totally possible!

[ianschmitz]

The output of ts.readConfigFile(configFile, ts.sys.readFile).config is the raw tsconfig.json, which would have compilerOptions, include, exclude, etc. I'm not sure how that would have worked previously 😕.

Regardless i can try an older version and do as you suggested tonight!

[johnnyreilly]

Regardless i can try an older version and do as you suggested tonight!

If you could I'd appreciate it!

cc @piotr-oles

It's possible that this wasn't picked up as the default value of isolatedModules is false...

https://www.typescriptlang.org/docs/handbook/compiler-options.html

Huh... I had completely forgotten I was the cause of all this in the first place!

See: #17

And: #18

[johnnyreilly]

Okay - having reread all that (isn't it amazing what you forget?!) I'm coming to the conclusion that losing this from the codebase is just fine I think.

If you could still do that check I'd appreciate it. Odds on I'd say this should be good to merge.

[ianschmitz]

Alright here's the results @johnnyreilly:

I added in a few console.log() entries on 0.4.10. From what i can see the output is what we expected. The isolatedModules: "false" is up a level too high, and should be inside the compilerOptions to have an effect:

output from ts.readConfigFile() isolatedModules hasnt been applied yet:

{ extends: './tsconfig.test.json',
  compileOnSave: false,
  compilerOptions:
   { target: 'es5',
     allowJs: true,
     skipLibCheck: true,
     esModuleInterop: true,
     allowSyntheticDefaultImports: true,
     strict: true,
     moduleResolution: 'node',
     resolveJsonModule: true,
     isolatedModules: true, <=======
     noEmit: true,
     jsx: 'preserve' } }


output from ts.parseJsonConfigFileContent() note the 'raw' property:

{ options:
   { module: 6,
     target: 1,
     allowJs: true,
     skipLibCheck: true,
     esModuleInterop: true,
     allowSyntheticDefaultImports: true,
     strict: true,
     moduleResolution: 2,
     resolveJsonModule: true,
     isolatedModules: true, <=======
     noEmit: true,
     jsx: 1,
     configFilePath: undefined },
  fileNames:
   [ 'C:/Projects/create-react-app/packages/react-scripts/template/src/App.js',
     'C:/Projects/create-react-app/packages/react-scripts/template/src/index.tsx',
     'C:/Projects/create-react-app/packages/react-scripts/template/src/react-app.d.ts',
     'C:/Projects/create-react-app/packages/react-scripts/template/src/serviceWorker.d.ts',
     'C:/Projects/create-react-app/packages/react-scripts/template/src/serviceWorker.js' ],
  projectReferences: undefined,
  typeAcquisition: { enable: false, include: [], exclude: [] },
  raw:
   { extends: './tsconfig.test.json',
     compileOnSave: false,
     compilerOptions:
      { target: 'es5',
        allowJs: true,
        skipLibCheck: true,
        esModuleInterop: true,
        allowSyntheticDefaultImports: true,
        strict: true,
        moduleResolution: 'node',
        resolveJsonModule: true,
        isolatedModules: true, <=======
        noEmit: true,
        jsx: 'preserve' },
     isolatedModules: false, <=======
     include: [ 'src' ],
     exclude: [ '**/__tests__/**', '**/?*test.*', '**/?*spec.*', 'foo' ] },
  errors: [],
  wildcardDirectories:
   { 'C:/Projects/create-react-app/packages/react-scripts/template/src': 1 },
  compileOnSave: false,
  configFileSpecs:
   { filesSpecs: undefined,
     includeSpecs: [ 'src' ],
     excludeSpecs: [ '**/__tests__/**', '**/?*test.*', '**/?*spec.*', 'foo' ],
     validatedIncludeSpecs: [ 'src' ],
     validatedExcludeSpecs: [ '**/__tests__/**', '**/?*test.*', '**/?*spec.*', 'foo' ],
     wildcardDirectories:
      { 'C:/Projects/create-react-app/packages/react-scripts/template/src': 1 } } }

[ianschmitz]

Would also explain this comment: #17 (comment)

[johnnyreilly]

Thanks @ianschmitz - I'm convinced! 😁