Handle output of TS compiler for JS and JSON files

[bartlomieju]

Currently TS compiler during compilation of .ts file will compile all encountered .js and .json files. For each file it produces compiled file (which is identical to original file) and source map file.

They are stored inside $DENO_DIR/gen/ directory. These files are actually never used because when Loader requests module it won't be requested using TS compiler because they are not .ts files.

This issue is related to #2114

Proposed solution:

1. remove resolveJsonModule from tsconfig.json or don't cache compiler output for .json files
2. don't cache compiler output for .js files unless checkJs is enabled in tsconfig.json - this would have to be done in coordination with Cleanup compiler pipeline #2657 because then TS compiler would be able to return compiled file for .js file

CC @ry @kitsonk

[kitsonk]

I would strongly object removing resolveJsonModule from the compiler options. This removes the ability for the compiler to enforce strong typing of import files. Not caching the output would be acceptable, IMO. I would recommend that it gets stripped out in the compiler results someway, before the compiler response gets serialised, so we simply "trust" that everything that is returned from the compiler should be cached. One thing, after Ry's compiler rewrite to support ESM that I never did spend understanding is how is imported JSON injected into the runtime? Is it more efficient to just take the string and dump it into the isolate? When we were doing AMD, the compiler itself would generate a .js file that was an AMD module the deserialised the JSON in its factory. It would think it maybe worth considering allow the compiler to do that, so there is a clear seperation of concerns that the compiler takes all input assets being imported and outputs valid JSON. If it is truly more efficient to have that just dumped into the isolate some other way, I think the design needs to address the logical flow that an extensions/mime-type has a "compiler". The compiler may request n other resources to generate a compilation, and the generated compilation is cached. The main module gets injected in the isolate and pulls in what ever other JS files that are then requested, hopefully all of them coming from the cache, because it was generated by the compiler.

The checkJs has not been fully re-enabled. I believe I mentioned before somewhere that I was concerned about the future because it is legitimate that the compiler can take in an input of .js and output .js that is different. That is legitimate for our current compiler and legitimate for other userland compilers when we add the feature. The caching of modules should accomodate that the input is one file and the output is a .js file, allowing for the input to be of any type including .js.

[bartlomieju]

Thanks for the reply @kitsonk, I have some questions though:

I would strongly object removing resolveJsonModule from the compiler options. This removes the ability for the compiler to enforce strong typing of import files.

Can you elaborate on this bit? https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-9.html#new---resolvejsonmodule
Yeah, resolveJsonModule definitely stays

Not caching the output would be acceptable, IMO. I would recommend that it gets stripped out in the compiler results someway, before the compiler response gets serialised, so we simply "trust" that everything that is returned from the compiler should be cached.

Sounds good to me, we just need to figure out how to decide if something should be cached or discarded.

One thing, after Ry's compiler rewrite to support ESM that I never did spend understanding is how is imported JSON injected into the runtime?

Currently it is pretty bad, this bit handles JSON:

deno/cli/deno_dir.rs

Lines 46 to 60 in 621af21

	pub fn js_source(&self) -> String {
	if self.media_type == msg::MediaType::TypeScript {
	panic!("TypeScript module has no JS source, did you forget to run it through compiler?");
	}
	// TODO: this should be done by compiler and JS module should be returned
	if self.media_type == msg::MediaType::Json {
	return format!(
	"export default {};",
	str::from_utf8(&self.source_code).unwrap()
	);
	}
	// it's either JS or Unknown media type
	str::from_utf8(&self.source_code).unwrap().to_string()

Is it more efficient to just take the string and dump it into the isolate? When we were doing AMD, the compiler itself would generate a .js file that was an AMD module the deserialised the JSON in its factory. It would think it maybe worth considering allow the compiler to do that, so there is a clear seperation of concerns that the compiler takes all input assets being imported and outputs valid JSON.

Currently TS compiler doesn't perform any trasformation on .json files, ie. output of compiler is the same as input file.

I believe we might use the trick presented here: https://twitter.com/mathias/status/1143551692732030979

If it is truly more efficient to have that just dumped into the isolate some other way, I think the design needs to address the logical flow that an extensions/mime-type has a "compiler". The compiler may request n other resources to generate a compilation, and the generated compilation is cached. The main module gets injected in the isolate and pulls in what ever other JS files that are then requested, hopefully all of them coming from the cache, because it was generated by the compiler.

This issue is to be addressed in #2657

The checkJs has not been fully re-enabled. I believe I mentioned before somewhere that I was concerned about the future because it is legitimate that the compiler can take in an input of .js and output .js that is different. That is legitimate for our current compiler and legitimate for other userland compilers when we add the feature. The caching of modules should accomodate that the input is one file and the output is a .js file, allowing for the input to be of any type including .js.

With #2636 landed it is now possible and quite trivial to implement, again we just need some logic to decide whether certain file should be compiled using tsc or not.

Can I assume that unless checkJs is set to true we should discard compiler output for .js files and discard it always for .json files?

[kitsonk]

Currently TS compiler doesn't perform any trasformation on .json files, ie. output of compiler is the same as input file.

That is right, the TypeScript compiler doesn't, but the Deno compiler used to, before the ESM. It would do exactly what is suggested in the link you sent:

For something like this:

{ "foo": "bar" }

You would get:

define("foo.json", [], function () {
  return JSON.parse(`{ "foo": "bar" }`);
});

The problem I just realised is that it isn't possible to generate a module that spreads the results of a string. Each export (key of the imported JSON) of the parsed JSON has to be available statically in ESM. The only real way to do it I think in ESM is how Ry did it. 😢 The other way is the compiler actually parses the JSON, enumerates over the keys and creates an export statement for each one.

Can I assume that unless checkJs is set to true we should discard compiler output for .js files and discard it always for .json files?

Yes, sounds right to me. And changes to the source .js should invalidate the cache when checkJs is true.

[bartlomieju]

Closed in #2686, not sure why it can't close two issues