Add support for loading tsconfig.json #2089

kitsonk · 2019-04-10T05:00:18Z

Resolves #51

This PR supports the command line option -c/--config and a tsconfig.json file to be passed which will then be used by the compiler.

I need to do a couple things, plus any feedback on the PR:

finish tests
~~detect "checkjs": true in a configuration and send JS files to the compiler to re-enable the support of CheckJS for Deno.~~
when caching, add the configuration to the fingerprint so that using a different config file settings will invalidate the cache.

js/compiler.ts

ry · 2019-04-11T14:23:01Z

Cool!

I’m looking at the failure on appveyor and don’t understand why expect != actual

tests/config.test ... FAILED
2619Expected output does not match actual.
2620Expected output:
2621Unsupported compiler options in "[WILDCARD]tests/config.tsconfig.json"
2622 The following options were ignored:
2623 module, target
2624Compiling file://[WILDCARD]tests/config.ts
2625[WILDCARD]tests/config.ts:3:5 - error TS2532: Object is possibly 'undefined'.
2626
26273 if (map.get("bar").foo) {
2628 ~~~~~~~~~~~~~~
2629
2630
2631Actual output:
2632Unsupported compiler options in "\?\C:\deno\tests\config.tsconfig.json"
2633 The following options were ignored:
2634 module, target
2635Compiling file:///C:/deno/tests/config.ts
2636C:/deno/tests/config.ts:3:5 - error TS2532: Object is possibly 'undefined'.
2637
26383 if (map.get("bar").foo) {
2639 ~~~~~~~~~~~~~~
2640
2641
2642

zekth · 2019-04-11T23:49:35Z

@ry I think the handling of wildcard is not the same in Linux/Windows. Maybe we have to do a workaround for those environment as it's due to tsc behavior and not deno. Or for the moment just only check the exit code (i know it's not perfect :/ )

hayd · 2019-04-12T00:17:54Z

+     actual_out = actual_out.replace(os.path.sep, '/')

above or before here, should fix, but might be overkill...

zekth · 2019-04-12T00:37:59Z

+     actual_out = actual_out.replace(os.path.sep, '/')
above or before here, should fix, but might be overkill...

it will be more complex than that look at [WILDCARD] part

kitsonk · 2019-04-14T07:46:35Z

Yeah, I caught the appveyor as well. I am changing a couple things in the PR today and updating and fixing the appveyor failure as well.

I think the CheckJS should be another PR after this lands as it would require a lot of infrastructure. Also, I am going to load the configuration differently, in order to facilitate both CheckJS but also the cache invalidation, but that could be a subsequent PR as well.

kitsonk · 2019-04-17T19:27:01Z

Sorry it is taking so long, I am travelling with family and don't have as much time as I would expect.

I have moved the loading of the config directly into the thread safe state, which should make it easier to be able to add the config to the hash and detecting if CheckJS is enabled.

cli/compiler.rs

js/compiler.ts

cli/state.rs

js/compiler.ts

kitsonk · 2019-04-25T10:13:08Z

Sorry, still travelling, but I should get some time over the next couple days to finish this off.

kitsonk · 2019-04-26T11:02:34Z

Testing the state changes in isolation is really challenging, because I am not sure of the easiest way to "stub" reading of the file. I haven't added a test for that yet.

I still want to do the cache invalidation as part of this PR, which I should be able to address in the next 24 hours.

zekth · 2019-04-26T11:17:22Z

Testing the state changes in isolation is really challenging, because I am not sure of the easiest way to "stub" reading of the file. I haven't added a test for that yet.

I still want to do the cache invalidation as part of this PR, which I should be able to address in the next 24 hours.

What about generating a temporary file to be read by the test?

kitsonk · 2019-04-26T16:01:44Z

What about generating a temporary file to be read by the test?

Can you point me to any examples of something like that in Rust?

zekth · 2019-04-26T20:32:59Z

What about generating a temporary file to be read by the test?

Can you point me to any examples of something like that in Rust?

You can use TempirDir: https://docs.rs/tempdir/0.3.7/tempdir/
And pass its path to DenoDir as a customroot ? But as mentionned creates a directory on the file system that is deleted once it goes out of scope so all the test as to be scoped.

Or do the same behaviour by writing your own func in the test.

kitsonk · 2019-04-27T10:59:18Z

Ok, the cache invalidation is added... This is now ready for a proper review @ry.

ry

Looking good! A few comments...

cli/deno_dir.rs

cli/state.rs

tests/config.test

js/compiler.ts

bartlomieju · 2019-04-28T17:06:03Z

cli/flags.rs

+        .long("config")
+        .value_name("FILE")
+        .help("Load compiler configuration file")
+        .takes_value(true),


Just feed for thought here: If we ever find a need to introduce deno specific config, we'll have trouble with appropriate flag name, how about --ts-config for TS compiler configuration?

Personal opinion, if we ever need a custom config file for Deno, we are probably doing it wrong. Even if we allowed setting of flags via a configuration file, I would hope we would expand the syntax of the TypeScript format, where "compilerOptions" would still be provided to the compiler and the rest parsed by Deno. In my opinion, Deno and TypeScript should always be synergistic instead of some sort of "adjunct".

Deno and TypeScript should always be synergistic instead of some sort of "adjunct".

Fits the Philosphy. Thumb up.

@kitsonk thanks for response, I generally agree with your opinion - now looking at #1739 it makes a lot of sense to make it single --config that other compilers can use.

ry · 2019-04-29T14:57:16Z

js/colors.ts

+// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
+
+// TODO(kitsonk) Replace with `deno_std/colors/mod.ts` when we can load modules
+// which end in `.ts`.


ry

LGTM - Thanks @kitsonk - this is a long time coming : )

irustm reviewed Apr 10, 2019

View reviewed changes

js/compiler.ts Outdated Show resolved Hide resolved

bartlomieju reviewed Apr 10, 2019

View reviewed changes

js/compiler.ts Show resolved Hide resolved

agentframework mentioned this pull request Apr 11, 2019

Add deno support agentframework/agentframework#10

Open

This was referenced Apr 13, 2019

[WIP] Deno_std Browser testing denoland/std#335

Closed

doc: Add missing documentation denoland/std#336

Merged

kitsonk mentioned this pull request Apr 14, 2019

Ability to support CheckJS #2114

Closed

kitsonk force-pushed the tsconfig branch from 4663ed9 to 2dd35ee Compare April 17, 2019 19:24

ry reviewed Apr 17, 2019

View reviewed changes

cli/compiler.rs Show resolved Hide resolved

ry reviewed Apr 17, 2019

View reviewed changes

js/compiler.ts Show resolved Hide resolved

ry reviewed Apr 17, 2019

View reviewed changes

js/compiler.ts Show resolved Hide resolved

bartlomieju mentioned this pull request Apr 20, 2019

Ability to modify TS config #2155

Closed

afinch7 reviewed Apr 24, 2019

View reviewed changes

cli/state.rs Outdated Show resolved Hide resolved

js/compiler.ts Show resolved Hide resolved

kitsonk force-pushed the tsconfig branch from 2dd35ee to 81d86e3 Compare April 26, 2019 10:59

kitsonk changed the title ~~[WIP] Add support for loading tsconfig.json~~ Add support for loading tsconfig.json Apr 27, 2019

ry reviewed Apr 27, 2019

View reviewed changes

cli/deno_dir.rs Show resolved Hide resolved

cli/deno_dir.rs Show resolved Hide resolved

cli/state.rs Show resolved Hide resolved

tests/config.test Show resolved Hide resolved

js/compiler.ts Outdated Show resolved Hide resolved

kitsonk added 2 commits April 28, 2019 11:08

Add support for loading tsconfig.json.

fbb8337

Config changes invalidate cache.

daa4257

kitsonk force-pushed the tsconfig branch from 1cf619d to dbff20a Compare April 28, 2019 10:32

bartlomieju reviewed Apr 28, 2019

View reviewed changes

kitsonk force-pushed the tsconfig branch from dbff20a to dd61a9e Compare April 29, 2019 11:30

Address feedback.

03b8280

kitsonk force-pushed the tsconfig branch from dd61a9e to 03b8280 Compare April 29, 2019 11:43

ry reviewed Apr 29, 2019

View reviewed changes

ry approved these changes Apr 29, 2019

View reviewed changes

ry merged commit 1a0f53a into denoland:master Apr 29, 2019

kitsonk deleted the tsconfig branch June 4, 2019 06:37

rsp mentioned this pull request Jan 15, 2020

Public API for compilers #1739

Open

Dan503 mentioned this pull request Jan 26, 2020

deno bundle does not support tsconfig.json #3793

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for loading tsconfig.json #2089

Add support for loading tsconfig.json #2089

kitsonk commented Apr 10, 2019 •

edited

Loading

ry commented Apr 11, 2019 •

edited

Loading

zekth commented Apr 11, 2019 •

edited

Loading

hayd commented Apr 12, 2019

zekth commented Apr 12, 2019

kitsonk commented Apr 14, 2019

kitsonk commented Apr 17, 2019

kitsonk commented Apr 25, 2019

kitsonk commented Apr 26, 2019

zekth commented Apr 26, 2019

kitsonk commented Apr 26, 2019

zekth commented Apr 26, 2019 •

edited

Loading

kitsonk commented Apr 27, 2019

ry left a comment

bartlomieju Apr 28, 2019

kitsonk Apr 29, 2019

zekth Apr 29, 2019

bartlomieju Apr 29, 2019

ry Apr 29, 2019

ry left a comment

Add support for loading tsconfig.json #2089

Add support for loading tsconfig.json #2089

Conversation

kitsonk commented Apr 10, 2019 • edited Loading

ry commented Apr 11, 2019 • edited Loading

zekth commented Apr 11, 2019 • edited Loading

hayd commented Apr 12, 2019

zekth commented Apr 12, 2019

kitsonk commented Apr 14, 2019

kitsonk commented Apr 17, 2019

kitsonk commented Apr 25, 2019

kitsonk commented Apr 26, 2019

zekth commented Apr 26, 2019

kitsonk commented Apr 26, 2019

zekth commented Apr 26, 2019 • edited Loading

kitsonk commented Apr 27, 2019

ry left a comment

Choose a reason for hiding this comment

bartlomieju Apr 28, 2019

Choose a reason for hiding this comment

kitsonk Apr 29, 2019

Choose a reason for hiding this comment

zekth Apr 29, 2019

Choose a reason for hiding this comment

bartlomieju Apr 29, 2019

Choose a reason for hiding this comment

ry Apr 29, 2019

Choose a reason for hiding this comment

ry left a comment

Choose a reason for hiding this comment

kitsonk commented Apr 10, 2019 •

edited

Loading

ry commented Apr 11, 2019 •

edited

Loading

zekth commented Apr 11, 2019 •

edited

Loading

zekth commented Apr 26, 2019 •

edited

Loading