fix: use get-tsconfig to handle tsconfig file resolution #247
Conversation
| "ts-node": "^10.7.0", | ||
| "typescript": "^4.5.2" | ||
| "typescript": "^5.0.3" |
There was a problem hiding this comment.
need to update typescript because get-tsconfig uses the new .d.cts and .d.mts file extensions which were not supported by TypeScript v4.5.2
| "lint-staged": "^10.2.11", | ||
| "prettier": "^2.0.5", | ||
| "rimraf": "^2.6.2", | ||
| "ts-jest": "^27.0.7", | ||
| "ts-jest": "^29.1.0", |
There was a problem hiding this comment.
updating ts-jest for compatibility with typescript
| @@ -32,18 +32,17 @@ | |||
| "eslint-plugin-import": "^2.26.0", | |||
| "eslint-plugin-jsdoc": "^39.2.9", | |||
| "husky": "^4.2.5", | |||
| "jest": "^27.3.1", | |||
| "jest": "^29.5.0", | |||
There was a problem hiding this comment.
updating jest for compatibility with ts-jest
|
hey @jonaskello following up in this PR as well just to get your attention. let me know if there's anything I can do to help move either of these changes forward |
|
Hi @Jontem sorry to ping you as well, but I wasn't getting a response from your colleague Unfortunately your library is at the root of many typescript-enabled workflows and this issue is seriously affecting my team because it does not follow the extension behaviour of official TypeScript tools |
Yes, I know this :-). So the main problem or merging anything is that we don't want to break any of the many existing users, which makes larger changes like this difficult to assess. I saw that a bunch of tests were removed. I guess this was because the assumption is that the external library is fully tested? But perhaps we can keep all current tests to ensure some level of combability? |
|
Thanks for the reply @jonaskello :) sorry for the continual pings on these changes and sure 👍 I'm happy to re-add the tests for added safety Do you have any preference between using |
|
I don't like relying on undocumented API:s so although I don't like external dependencies either I guess |
|
Amazing 👍 I'll rework this PR to keep the existing tests as best as possible |
|
Heya @jonaskello long time no chat 😅 As you can see it maintains compatibility with the original test suite while also handling multiple levels of extension (the original reason for this PR) Let me know what else I can do to push this along, I'll try to follow up more regularly going forward |
| const res = loadTsconfig( | ||
| "/root/dir1/tsconfig.json", | ||
| (path) => path === "/root/dir1/tsconfig.json", | ||
| (_) => `{ |
There was a problem hiding this comment.
since the API from get-tsconfig does not support a resolve callback like this, I opted instead to create entries in the example/ folder for each test case
| }); | ||
| }); | ||
|
|
||
| describe("getTsconfig", () => { |
There was a problem hiding this comment.
I extracted these tests which only target the getTsconfig function from get-tsconfig to show that other details like strict are correctly inherited (since this information is not returned from loadSyncDefault)
|
Nice work! However, it seems the github checks are failing, perhaps you can take a look at it? |
will do 👍 |
hey @jonaskello I rewrote my other PR using the library
get-tsconfigwhich does not rely on the API exposed bytypescript, it has zero dependencies and implements all of the features of the latest TSConfig files (including extends as an array, which I see was a recent change to this repo).I'd really appreciate some feedback on how to move either of these changes forward