resolveJsonModule with --module amd

[LeviticusMB]

TypeScript Version: 3.1.0-dev.20180717

Search Terms: #25517

Code

#!/bin/bash

set -x
rm -rf node_modules package*.json import-json.ts bundle*js

npm init --yes
npm add [email protected] [email protected]

cat > 'import-json.ts' <<EOF
import phoneMetadata from 'libphonenumber-js/metadata.mobile.json';

console.log(phoneMetadata)
EOF

npx tsc --esModuleInterop --resolveJsonModule --outFile bundle-2.9.2.js --module amd --moduleResolution node import-json.ts

npm add typescript@next
npx tsc --version
npx tsc --esModuleInterop --resolveJsonModule --outFile bundle-next.js --module amd --moduleResolution node import-json.ts

Expected behavior:

The code that used to work just fine in 2.9 still works in 3.x without generating compilation errors.

Actual behavior:

The resolution of #25517 (which I'm really not happy with) not only did not fix the issue reported, it also broke our existing code referencing a json file from the libphonenumber-js module.

Playground Link:

Related Issues: #25517

[mhegazy]

The code that was emitted before for --module amd was not right. using require to load the .json file would fail at run time. I am not sure i understand how that worked for you previously. can you elaborate?

[LeviticusMB]

Well it' not any different from importing the libphonenumber-js library itself: We just have a build script that makes sure the module and the data files are available at runtime.

import phoneNumber from 'libphonenumber-js';
import phoneMetadata from 'libphonenumber-js/metadata.mobile.json';

console.log(phoneNumber.isValidNumber('012-345678', 'SE'));
console.log(phoneMetadata);

(The only quirk was that when targeting amd instead of commonjs or es6, the JSON module name becomes node_modules/libphonenumber-js/metadata.mobile and not libphonenumber-js/metadata.mobile.json as one might have expected.)

[mhegazy]

We just have a build script that makes sure the module and the data files are available at runtime.

aha! that is how it is working 😃. well the idea is the output the compiler should not generate invalid code silently , and not all users have a script as such, nor there is a way to tell them they need to have one. most AMD users i am aware of use a json! plugin to load .json files.

[LeviticusMB]

So I'm not allowed to import modules from node_modules at all when the output format is amd??

Because there is no is no difference between import phoneNumber from 'libphonenumber-js' and import phoneMetadata from 'libphonenumber-js/metadata.mobile.json': Both will generate code that requests a module that that is not present in the output bundle and will require additional build support to run correctly in the browser. That's just how AMD (or any other platform except NodeJS, actually) works. The tsc-generated code is not invalid.

--resolveJsonModule was a great flag because it gave us great tooling support in VS Code. I don't understand why we should have to revert back to global d.ts hacks again, just because we compile to a platform that is not NodeJS.

It used to work, the code generated was certainly just as valid as any other module imported from node_modules (like @angular, for instance), and now it's broken in @next.

[domoritz]

I filed a separate issue at #26020 for supporting JSON with es2015 modules. I really hope that this changes because otherwise typescript and rollup won't work well together if I want to also get things like the version from a JSON file.

[Soul-Master]

resolveJsonModule config should supports amd module that should allow us to use rollup-plugin-json plugin to import it and bundle with other modules.

In TypeScript 2.9.1, everything work perfectly fine. To make my project works in TypeScript 3, I may need to use global definition for JSON module again.

[domoritz]

Unfortunately, I have to use TS 3 because of a bug in TS 2.9 where the full import path appears in the type declarations. This was fixed in TS 3 and not backported to TS 2.9. This is fine except for this issue with JSON imports.

[paavohuhtala]

We also need this, or actually support for --module esnext to be precise. Our frontend bundle size grew significantly (almost doubled) after upgrading to TS 3.0, because Webpack's dead code elimination only works with ES6 module imports. We can't downgrade to 2.9, because we've already adopted composite projects. Luckily we are not in production yet, but I hope this will be supported soon.

[domoritz]

@mhegazy do you need more info for this issue? I'd be happy to contribute more examples.

[sheetalkamat]

Adding @RyanCavanaugh and @DanielRosenwasser to this issue to give their opinion and suggest what needs to be done.

[jpike88]

IMO this should be considered a regression and treated as a bug. The following error check is in my way now, and I don't want to comb through my project to render it commonjs friendly when it worked fine with JSON imports till this point.

https://github.com/Microsoft/TypeScript/blob/d8cbe34a051a64d39f55f4e46e18ac404aac922e/src/compiler/diagnosticMessages.json#L2887

[ryanelian]

resolveJsonModule should be usable with esnext module because it lets seamless integration between TypeScript static checking and webpack's native JSON module...

[jpike88]

@mhegazy why is this still marked as 'needs more info'? Can someone update this issue accordingly?

Should we just remove the offending error check?

[domoritz]

@mhegazy @sheetalkamat @RyanCavanaugh and @DanielRosenwasser, can you give us an estimate of when this issue will be looked at? I hate to be naggy about this but I'm kind of in a limbo state with a lot of libraries right now. I had to update to TS 3 because of a bug in TS 2.9 but now the builds don't work anymore because of this issue (rollup doesn't produce correct code when commonjs) and I don't know whether this issue here is TS is by design and I need to find another way to import from JSON. Regardless, you are all doing amazing work and I can't thank you enough for TS!

[jpike88]

So it looks increasingly that just to stay up to date with typescript, I have to convert 4 projects to 'commonjs' or be stuck on 2.9 indefinitely. Do I really, really want to go there? Please can someone who is knowledgable give me an indication on where this issue is at? A clear answer is all I'm looking for...

[domoritz]

An alternative to switching to commonjs is to add a json typings file. You won't get type safety but at least you can build again.

declare module "*.json" {
    const value: any;
    export default value;
}

[mlg87]

can confirm that @domoritz solution works for me

[domoritz]

Having said that, the approach has a few disadvantages. First, you don't get type checks. Second, you might have to copy your package.json to the right location for your build system to pick it up. Lastly, you have to include the JSON typings file in all dependent projects that also use Typescript.

[jpike88]

I don’t get it. It was working for Esnext users and then it was broken. It’s a regression regardless of whether esnext is tracking or not, as entire projects are structured on this. This reversal requires that a fair amount of technical investment is done to satisfy a completely arbitrary restriction.

@RyanCavanaugh
Your thoughts are much appreciated because I and clearly a bunch of others are stuck on 2.9.2 with a very impractical way forward.

[jpike88]

This issue is getting ignored and I’m about to freeze our ts version indefinitely as well as unsubscribe from typescript issues. If this won’t be graced with a response, the status quo will do fine.

[weichensw]

I am on the same boat, --module esnext with --resolveJsonModule. As a workaround, I currently disabled --resolveJsonModule and use scripts to automatically generates somefile.json.d.ts for all my JSON files so that I could keep the codebase working. Though it requires an extra compilation pass and things get rather complicated with watch, etc.

[RyanCavanaugh]

Sorry for the confusion here, everyone. We didn't intend to support this scenario (the number of different flag combinations we have to think about is overwhelming!) and ended up shipping something we didn't realize was working for some subset of people.

The behavior described in #25517 (comment) is what we should do now that people have taken a dependency on it. It's too far past 3.0 to backport the fix at this point, so I'd advise people to stay on 2.9 and move directly to 3.1 once it's available. Thanks!

[domoritz]

Thank you @sheetalkamat and @RyanCavanaugh for the fix. I look forward to the 3.1 release.

[LeviticusMB]

Really appreciate the fix. Thanks all! 🥇