fix: use local gts path for generated tsconfig.json #114
Conversation
|
One test will fail, which is being fixed by #112 |
|
This fixes #113. |
Currently the global gts path is used when `gts init` is done using a globally installed `gts`, which is a usual workflow.
| const opts = {cwd: `${tmpDir.name}/kitchen`}; | ||
| // It's important to use `-n` here because we don't want to overwrite | ||
| // the version of gts installed, as it will trigger the npm install. | ||
| await simpleExecp(`${GTS} init -n`, opts); |
There was a problem hiding this comment.
I changed -n to -y because a local gts must be installed so that the gts check below can find the tsconfig-google.json file.
|
As we discussed, I moved the test to 'use as a non-locally installed module'. |
Codecov Report
@@ Coverage Diff @@
## master #114 +/- ##
==========================================
+ Coverage 96.6% 96.63% +0.03%
==========================================
Files 10 10
Lines 324 327 +3
Branches 19 19
==========================================
+ Hits 313 316 +3
Misses 11 11
Continue to review full report at Codecov.
|
| const tsconfig = formatJson({ | ||
| extends: baseConfig, | ||
| extends: './node_modules/gts/tsconfig-google.json', |
There was a problem hiding this comment.
Should path.join be used instead of using / directly?
There was a problem hiding this comment.
I don't think we should have a different value here depending on on which platform the source code is generated, right?
There was a problem hiding this comment.
I'm not sure. I would expect tsconfig to be like package.json where we can always use /, but we should probably make sure.
There was a problem hiding this comment.
Windows has always supported / as a path separator as well as \. So I don't see any issues with this if we consider all the current major OS's.
| await simpleExecp(`${GTS} init -n`, opts); | ||
| await simpleExecp(`${GTS} init -y`, opts); | ||
| // The `extends` field must use the local gts path. | ||
| const tsconfigJson = |
There was a problem hiding this comment.
nit: Could you use pify to promisify readFile so that you can await the readFile? Also, should you be using path.join here and in the t.deepEqual line?
There was a problem hiding this comment.
This test file uses lots of ...Sync functions. I agree that using async functions is better but since this is a test file, I think we'd better make the code more consistent. WDYT?
There was a problem hiding this comment.
I agree its not that big of a deal since its a test file, especially if there are a lot of places where *Sync functions are used. If we wanted to make them async, it would probably be best as a follow-up PR.
Currently the global gts path is used when
gts initis done using aglobally installed
gts, which is a usual workflow.