Skip to content

Cannot compile with TS 2.9.1 & "resolveJsonModule" #793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mbenadda opened this issue Jun 12, 2018 · 16 comments
Closed

Cannot compile with TS 2.9.1 & "resolveJsonModule" #793

mbenadda opened this issue Jun 12, 2018 · 16 comments

Comments

@mbenadda
Copy link

mbenadda commented Jun 12, 2018

Expected Behaviour

TS 2.9 added a new option to allow JSON imports and get proper types doing so (without resorting to somewhat dirty workarounds using ambient definitions), with the resolveJsonModule compiler option.

It should compile as usual when using this new flag.

Actual Behaviour

An error is thrown when running webpack with ts-loader:

/Users/mehdibenadda/Development/fun-cms/node_modules/typescript/lib/typescript.js:107386
            return program.getOptionsDiagnostics(cancellationToken).concat(program.getGlobalDiagnostics(cancellationToken));
                           ^

TypeError: Cannot read property 'getOptionsDiagnostics' of undefined
    at Object.getCompilerOptionsDiagnostics (/Users/mehdibenadda/Development/fun-cms/node_modules/typescript/lib/typescript.js:107386:28)
    at provideCompilerOptionDiagnosticErrorsToWebpack (/Users/mehdibenadda/Development/fun-cms/node_modules/ts-loader/dist/after-compile.js:39:31)
    at /Users/mehdibenadda/Development/fun-cms/node_modules/ts-loader/dist/after-compile.js:17:9
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:16:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/Hook.js:35:21)
    at compilation.seal.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compiler.js:497:30)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeAssets.callAsync.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:985:35)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeChunkAssets.callAsync.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:976:32)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.additionalAssets.callAsync.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:971:36)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeTree.callAsync.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:967:32)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/Hook.js:35:21)
    at Compilation.seal (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:904:27)
    at hooks.make.callAsync.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compiler.js:494:17)
    at _err0 (eval at create (/Users/mehdibenadda/Development/fun-cms/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:11:1)
    at _addModuleChain (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:770:12)
    at processModuleDependencies.err (/Users/mehdibenadda/Development/fun-cms/node_modules/webpack/lib/Compilation.js:709:9)
    at process._tickCallback (internal/process/next_tick.js:150:11)

Steps to Reproduce the Problem

Compile a project using webpack, ts-loader and [email protected] with resolveJsonModule (and esModuleInterop) enabled and an import for a JSON file somewhere in the project.

Removing resolveJsonModule and using an ambient declaration works fine. I tried this with ts-[email protected] and typescript@next

Location of a Minimal Repository that Demonstrates the Issue.

https://github.com/mbenadda/ts-loader-issue-793-example-repo

See instructions in README, just clone, yarn install and yarn webpack to see the issue.

@johnnyreilly
Copy link
Member

Hey thanks for reporting and for the repro @mbenadda!

I'm afraid I'm snowed and so can't look at this right now. But I'm happy to help you dig into this. It looks like this is the code that's likely erroring: (compiled output of after-compile.ts)

function provideCompilerOptionDiagnosticErrorsToWebpack(getCompilerOptionDiagnostics, compilation, instance, configFilePath) {
    if (getCompilerOptionDiagnostics) {
        const { languageService, loaderOptions, compiler, program } = instance;
        const errorsToAdd = utils_1.formatErrors(program
            ? program.getOptionsDiagnostics() // I THINK THE ERROR IS OCCURRING HERE
            : languageService.getCompilerOptionsDiagnostics(), loaderOptions, instance.colors, compiler, { file: configFilePath || 'tsconfig.json' }, compilation.compiler.context);
        compilation.errors.push(...errorsToAdd);
    }
}

Can you confirm if that is the case? That program is undefined when it attempts to make that call with resolveJsonModule in play?

If so, the next question is "why"? Also, is program always undefined with resolveJsonModule in play?

Could you investigate? I'm wondering if with 2.9 there is now something extra that needs to be passed when creating a typescript.LanguageServiceHost / WatchHost in https://github.com/TypeStrong/ts-loader/blob/master/src/servicesHost.ts

To answer that, you may want ask a question of the TypeScript team. I'll help with advice / pointers etc - but I'm not going to be able to look at this personally right now I'm afraid.

@mbenadda
Copy link
Author

Hey @johnnyreilly thanks for your help!

I'll try to investigate but I'm not familiar with the internal structure of ts-loader nor typescript; and typescript is not easy to explore as it seems to use a lot of globals, so I apologize if I stumble on things that seem obvious to you or others. That said:

Can you confirm if that is the case? That program is undefined when it attempts to make that call with resolveJsonModule in play?

This is exactly what is happening.

is program always undefined with resolveJsonModule in play?

As far as I can see right now, no. It's only undefined when there is an actual import statement for a JSON module in the dependency tree. If I enable resolveJsonModule but do not import one there is no error.

I'm wondering if with 2.9 there is now something extra that needs to be passed when creating a typescript.LanguageServiceHost / WatchHost in https://github.com/TypeStrong/ts-loader/blob/master/src/servicesHost.ts

I started by checking required methods, arity and types and everything seems to match. There's no documented API breaking change in 2.9 either. I asked that question on the TS Gitter, will see if I get an answer there.

Might there be a problem due to ScriptKind.JSON which is probably a new ScriptKind in 2.9? ts-loader is not passing its own getScriptKind so it might be unrelated though. To be honest I'm not sure where to look next.

@johnnyreilly
Copy link
Member

johnnyreilly commented Jun 12, 2018

Thanks for looking into this @mbenadda

Might there be a problem due to ScriptKind.JSON which is probably a new ScriptKind in 2.9? ts-loader is not passing its own getScriptKind so it might be unrelated though. To be honest I'm not sure where to look next.

This sounds like it could be worth investigating. Be great to hear what comes out of the TS Gitter. Although she's in no way obligated to assist, I know that @sheetalkamat has often been really helpful around this area.

Related to that, it could be worth checking out other projects that hook into TypeScript. in the same way that ts-loader does. If they've recent commits related to TypeScript 2.9 that may prove interesting.

For example, it looks like fusebox might be facing the same issue: fuse-box/fuse-box#1288 (or related)

@johnnyreilly
Copy link
Member

I've just given your question a bump - hopefully that'll help!

@timocov
Copy link
Contributor

timocov commented Jun 12, 2018

Hi there,

It looks like this is the code that's likely erroring

According to the stack trace (at Object.getCompilerOptionsDiagnostics) it seems that the error happens at the line

: languageService!.getCompilerOptionsDiagnostics(),

@johnnyreilly
Copy link
Member

johnnyreilly commented Jun 13, 2018

Thanks @timocov! I was wondering: with resolveJsonModule in play, does languageService!.getCompilerOptionsDiagnostics() throw for every file? Or just json files?

It's worth bearing in mind that for each file in the program this code will likely be run. I'm interested as to whether it always fails regardless of file type when resolveJsonModule is true, or whether it's just json files....

Would someone be able to investigate?

@mbenadda
Copy link
Author

mbenadda commented Jun 13, 2018

I think I may have located the problem.

".json" is not in the list of supported extensions here: https://github.com/Microsoft/TypeScript/blob/master/src/parser/utilities.ts#L7805

Which causes that check to fail when trying to process a *.json file: https://github.com/Microsoft/TypeScript/blob/master/src/parser/utilities.ts#L7988


I'm not exactly sure why the tsc CLI itself does not choke on this. My guess would be that it parses differently through its own direct filesystem access and does not rely on this list of extensions, whereas API users need to pass lists of files which are then checked through this list.

I think this may relate to ts-loader through this call:

instance.languageService = compiler.createLanguageService(

Because when you look at createLanguageService source you find a call to createProgram right here: https://github.com/Microsoft/TypeScript/blob/master/src/services/services.ts#L1274

And in createProgram the failing check: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/program.ts#L585


Anyway, if I manually add ".json" to the supportedTypescriptExtensionsForExtractExtension list (not sure it should belong in either supportedTypeScriptExtensions or supportedJavascriptExtensions), compilation goes through and I actually get an output file that contains the JSON data as it should.

I'd like your opinion on this @johnnyreilly before I create an issue/attempt to make a PR on the TypeScript repo.

@johnnyreilly
Copy link
Member

Well done for investigating @mbenadda! I'm on a phone right now and so clicking on the code links is proving challenging...

One thing that strikes me is that when I look here: https://github.com/Microsoft/TypeScript/blob/master/src/parser/utilities.ts#L7805 I see json listed:

export function getScriptKindFromFileName(fileName: string): ScriptKind { 
// ...
case Extension.Json: return ScriptKind.JSON;
// ...
}

It's possible I've misunderstood you though. Phone and all that.

Anyway, if I manually add ".json" to the supportedTypescriptExtensionsForExtractExtension list (not sure it should belong in either supportedTypeScriptExtensions or supportedJavascriptExtensions), compilation goes through and I actually get an output file that contains the JSON data as it should.

This is by far and away the most interesting thing you've found. I think you should raise an issue against the Typescript repo linking back to this and listing your findings. I think you may have caught a bug.

Once again, well done for digging into this; I'm super grateful!

@mbenadda
Copy link
Author

It's possible I've misunderstood you though. Phone and all that.

As far as I can see, the part I linked is not checking script kinds, just getting file extensions directly during module name resolution. As part of getting the extensions, it checks they're in the allowed lists of extensions.

This is by far and away the most interesting thing you've found. I think you should raise an issue against the Typescript repo linking back to this and listing your findings. I think you may have caught a bug.

Once again, well done for digging into this; I'm super grateful!

I really appreciate your help and availability on this! I will create an issue on there and hopefully we've gathered enough info to get some help from someone there!

@johnnyreilly
Copy link
Member

Great! Go you!

@johnnyreilly
Copy link
Member

johnnyreilly commented Jun 14, 2018

When this TypeScript PR gets shipped the issue should be resolved: microsoft/TypeScript#24959

sampsyo added a commit to cucapra/gator that referenced this issue Jun 27, 2018
Working around this bug in ts-loader and the TypeScript compiler for
now:
TypeStrong/ts-loader#793

It looks like the fix won't be released immediately, so I'm switching
instead to separately compile the TypeScript and *then* run webpack.
This can be reverted when TypeScript 3.0 is released.
@noahm
Copy link

noahm commented Jul 30, 2018

This issue should now be resolved with today's release of Typescript 3.0 😄

@johnnyreilly
Copy link
Member

Awesome!

@simonbuchan
Copy link

For google juice: You get the same error message for microsoft/TypeScript#26554.

It seems like both ts-loader and TS itself could handle internal TS error messages better: the problem here is that it's trying to get the error diagnostics when compilation fails, but that also goes through the failing path (since it thinks the lazy init hasn't happened yet). Should this be a new issue: to give more meaningful errors when TS dies?

@johnnyreilly
Copy link
Member

Yes I think so.

@dev-sna
Copy link

dev-sna commented Dec 14, 2018

If you're using vscode and have builtin TS plugins disabled, enabling those extensions will resolve the issue.
Worked for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants