#15 added basic tsconfig extends support #16
Conversation
|
There's a normal I don't think I added linting to this project 😅 |
index.ts
Outdated
| }, | ||
| }; | ||
| const { compilerOptions } = getTsConfig(tsConfigPath); | ||
| const outDir = compilerOptions.outDir ?? '.'; |
There was a problem hiding this comment.
This was pointed out here already: #14 (comment)
There was a problem hiding this comment.
Right in my case, outDir was always the default (".") since I've defined it in my compilerOptions -> never was read in.
Since the resolvedFileName is already an absolute path, joining (with path.join()) it with the default outDir (".") resulted in the correct absolute path, as resolvedFileName seems already correct?
Example resolvedFileName: "C:/path/to/my/monorepo/packages/dtif-to-svg/src/d3.ts"
However, now that it reads in the outDir correctly due to your suggested getParsedCommandLineOfConfigFile() method that works like a charm, the outDir is something like "C:/path/to/my/monorepo/packages/dtif-to-svg/dist" which I defined in the compilerOptions of my tsconfig.json. However, now path.join() will join these both absolute paths that result in: "C:/path/to/my/monorepo/packages\dtif-to-svg\dist\C:/path/to/my/monorepo/packages\dtif-to-svg\src\d3.ts". And thats not correct.
-> For me it works best without any defined outDir as resolvedFileName already points to the correct location
-> Do we actually need to define the outDir and when might it be required, because for me it isn't?
There was a problem hiding this comment.
In the same issue comments I think further down there's a solution with using path.relative instead of path.join.
There was a problem hiding this comment.
yeah I tested that but the resulting relative path was something like ../src/path/to/file and since the script was executed in the root of the package it navigated out of the package and tried to find the src/path/to/file.ts there..
There was a problem hiding this comment.
but anyway if the MR doesn't fit the project's philosophy that's ok too :) I've implemented your very useful plugin in my scripts and updated it to my project's needs (represented in this MR).. but I've not tested it in other environments and everyone has different requirements and expectations, which is of course hard to cover in a single project.. cheers :)
index.ts
Outdated
| // Try to resolve as a file relative to the current config | ||
| extendedConfigPath = path.join( | ||
| path.dirname(rootConfigPath), | ||
| config.extends |
There was a problem hiding this comment.
extends can actually be an array since TS 5.0. I'm pretty sure there's probably a better method from the TypeScript API to retrieve the tsconfig.
I wrote this a long time ago when I was still completely unfamiliar with the API 😅
There was a problem hiding this comment.
https://stackoverflow.com/a/70013087/2897426 maybe this is the correct solution
There was a problem hiding this comment.
that actually worked quite good :)
thanks for pointing that out.. did totally miss that 😅 |
… command and improve ts config read in
| @@ -63,15 +57,16 @@ export const typescriptPaths = ({ | |||
| return null; | |||
| } | |||
|
|
|||
| const targetFileName = join( | |||
| outDir, | |||
| // TODO: Do we need outDir as "resolvedFileName" is already correct absolute path | |||
There was a problem hiding this comment.
I think resolvedFileName is only the correct absolute path with Rollup 3 but not Rollup 2 or so.
| if (!configJson) { | ||
| return defaults; | ||
| // Read in tsconfig.json | ||
| const parsedCommandLine = ts.getParsedCommandLineOfConfigFile( |
There was a problem hiding this comment.
This might be a lot slower than what happened before, so probably it makes sense to memoize the getTsConfig function for performance. I can add this (hopefully i'll find some time to properly go through this PR in the next few weeks).
| outDir: string; | ||
| } | ||
| } & Omit<ts.CompilerOptions, 'outDir'>; |
There was a problem hiding this comment.
Shouldn't it be like
| } & Omit<ts.CompilerOptions, 'outDir'>; | |
| } & { compilerOptions: Omit<ts.CompilerOptions, 'outDir'> }; |
Anyway, I thought having outDir next to compilerOptions in the tsconfig was wrong in the first place?
|
Any progress with merging? Still an issue :-/ |
see #15
Note: Sry for the wrong formatting.. maybe add normal eslint & prettier config with
.prettierrcand.eslintrc:)