fix: correctly detect JS projects with type checking

[benmccann]

Closes #20595

What I did

Detected if jsconfig.json exists

I also did some minor cleanup by removing an unnecessary cwd usage: https://nodejs.org/api/fs.html#string-paths

Checklist

• Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
• Make sure to add/update documentation regarding your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Maintainers

• If this PR should be tested against many or all sandboxes,
  make sure to add the ci:merged or ci:daily GH label to it.
• Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

[JReinhold]

1. Is the assumption here, that if there's a jsconfig.json then there isn't also a tsconfig.json, and we're certain it's not a TS project? It sounds reasonable to me, but I just want to make sure.

2. This only works if the jsconfig.json is in the CWD right, is that always the case? What about monorepos, is it valid to have a jsconfig.json in a parent directory, that we would be missing here? Based on a quick glance at hasDependency it looks like we're already detecting package.json in parent directories, so maybe we should do the same for jsconfig.json

[benmccann]

Is the assumption here, that if there's a jsconfig.json then there isn't also a tsconfig.json, and we're certain it's not a TS project? It sounds reasonable to me, but I just want to make sure.

Yeah, I don't know what TypeScript would do if you have both, but you really shouldn't have both.

This only works if the jsconfig.json is in the CWD right, is that always the case? What about monorepos, is it valid to have a jsconfig.json in a parent directory, that we would be missing here? Based on a quick glance at hasDependency it looks like we're already detecting package.json in parent directories, so maybe we should do the same for jsconfig.json

Yes. I think even in a monorepo you always need a tsconfig in each project directory. They can all essentially be empty except for an extends declaration extending from a common one in the root monorepo, but I don't think you can just leave it out

[JReinhold]

Yes. I think even in a monorepo you always need a tsconfig in each project directory. They can all essentially be empty except for an extends declaration extending from a common one in the root monorepo, but I don't think you can just leave it out

I did some research and it looks like it's actually perfectly valid to not have a tsconfig.json in the project, as TypeScript will look up in parent paths until it finds one. I think we should support the same, and it looks pretty simple with the get-tsconfig package:

import { getTsconfig } from 'get-tsconfig'

const hasJsConfig = Boolean(getTsconfig('.', 'jsconfig.json'));

[benmccann]

Thanks for finding that. I've made that change

[JReinhold]

Tests are failing, it was of course too easy.

Basically our tests for the detection mocks out path and fs, which causes get-tsconfig to fail because they are incomplete mocks. The mocks were introduced in this commit by @yannbf.

I see two paths forward:

1. Fix/remove the mocks so that the tests won't fail anymore.
2. Go back to the simpler solution that just supports jsconfig.json in the current dir, with a TODO comment explaining the unsupported edge-case.

I'm okay with option 2, as some support for jsconfig.json is better than nothing. But either way I think we should add a test case for it in detect.test.ts.

[benmccann]

I've gone back to the old way and added a TODO. I'm not sure how to add a test for this

[ndelangen]

@JReinhold this seems fine to merge to me, WDYT?