Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type-check dependent files too #6

Open
gustavopch opened this issue May 1, 2020 · 11 comments
Open

Type-check dependent files too #6

gustavopch opened this issue May 1, 2020 · 11 comments

Comments

@gustavopch
Copy link
Owner

Problem

Currently, tsc-files will receive a list of files as arguments and will only type-check them.

Suppose you have the following two files:

// types.ts
export type User = {
  name: string
}
// index.ts
import { User } from './foo'

const user: User = {
  name: 'John Doe'
}

Everything OK. But now imagine you apply some change to types.ts that would break index.ts:

-   name: string
+   fullName: string

And then you run tsc-files with lint-staged, so that tsc-files will only receive the files that were changed (index.ts won't be type-checked because it wasn't changed). In that case, tsc-files won't alert you that index.ts is broken.

Possible solution

To fix that behavior, tsc-files should be able to type-check not only the files given as arguments but also all files that recursively depend on those.

@barroudjo
Copy link

barroudjo commented May 11, 2020

A way: https://github.com/dependents/node-dependency-tree
The walk back the generated dependency tree from your changed files to find the files you need to add.
EDIT: actually it's not gonna be that useful, because you'll have to walk all the way back you dependency tree to be sure there's not any problem caused by the changes, and if the project has a single entry file, you'll end up with all your files...
So the only case in which this is useful is when your project has multiple entry files.

@gustavopch
Copy link
Owner Author

@barroudjo I guess we need the opposite. Given the leaves of the tree, we should walk back through all parent nodes. Maybe something like:

  1. Read the given files and store their paths somewhere
  2. For each file, find the static imports and resolve them
  3. If there were static imports in the last step, pass the results to the step 1
  4. Return the stored paths

@barroudjo
Copy link

That's actually what I'm saying, when I say walk back I mean find parents (ie files that use this changed file), and continue this process and return the path of all the files found through this process.
And an easy (the only actually) way to do this is to use a dependency tree constructed by a library such as the one I suggested.
The problem as I explained in my edit is that in projects with only one entry file, you'll end up with all your files, so not that useful...

Explanation, with a simple project file dependency tree (file A is the entry point):

A => B => C => D
||
E => F

If only D has changed you'll end up with files [A, B, C, D] that have to be type-checked. And from what I understood, when typescript is given a file, it checks it and all its dependencies, which makes sense. So with this process all files will end up being type checked, including E and F.
The only case where this does not happen is if there are more than one entry points. Something like that:

A => B => C => D

E => F => G

And in this case if D has changed only [A, B, C, D] will end up being type checked, not [E, F, G]

@gustavopch
Copy link
Owner Author

@barroudjo Got it. I'm afraid you're right. In that case, is this tool even useful? 😟

@barroudjo
Copy link

Yes it is, because of that defect of typescript, where giving it a file in the cli makes it ignore the tsconfig.json.
For example in my case I want to compile a script, but not the whole project, and I want to use the configuration in my tsconfig.json. Then your tool is a life saver !
The only other way would be to create a tsconfig for each script I want to compile and run, and make it depend on the main tsconfig... which is not a clean solution.

@barroudjo
Copy link

Convinced @gustavopch ?

@gustavopch
Copy link
Owner Author

@barroudjo Yes, that makes sense. I've actually found out that my original "type-checking with lint-staged" use case can be achieved with TypeScript's --incremental flag — it's pretty fast due to the cache. But I can see how this tool can be useful to transpile specific files as you said.

@kibin
Copy link

kibin commented Sep 1, 2020

Any progress on this? Or is it just an idea?

@gustavopch
Copy link
Owner Author

@kibin Unfortunately not. I'm not currently using tsc-files in my own projects, so I won't be able to work on this. A PR would be welcome though.

@benwainwrightcinch
Copy link

benwainwrightcinch commented Jan 26, 2022

@barroudjo I think it might be possible to overcome the problem you describe.

Given your scenario:

A => B => C => D
||
E => F

In the situation where D has changed, you only really need to know two things:

  • Is there any type errors in D or any of its dependencies?
  • Where D has an export, are there any type errors in C where these imports are used
  • You don't really need to type check A, B, E or F because you know they haven't changed.

This might be possible with a little bit of trickery. I'm speculating a bit here; but since the compiler has a flag which allows you to compile modules without checking their dependencies (isolatedModules), we might be able to leverage this to achieve the intended outcome.

  • Firstly to do this, you need to know the whole dependency tree of the project, as has already been suggested. You can get this using the TypeScript compiler API (I doubt you need to use node-dependency-tree). It will take some time, but if you look at the diagnostic output of a large compilation you'll see that most of the time is spent type-checking; so parsing the source files and getting the AST should be pretty quick.
  • Then using the the AST, identify C (or any other parent nodes)
  • Now you have the AST, you should be able to programatically manipulate it into a state that is useful to us: each 'import' statement of D needs to be replaced with a TypeScript ambient declaration (declare statement) that matches the exact types of the export
  • Once you have done that, if we can then use the TypeScript compiler to compile the transformed module using isolatedModules mode, then we should get the result we need.

I'm making loads of assumptions here and I'm not saying its easy. But it might be possible.

@ctsstc
Copy link

ctsstc commented Mar 24, 2022

Just had this bite me -- changed a function signature but didn't update the implementation, but of course the implementation was not in the staged files so it passed. Running a tsc against the whole project would have caught it.

Essentially we need what Jest has the --findRelatedTests flag.

Seems like the related code is here:

I was kind of hoping they used a 3rd party AST library, but it seems that they've baked in their own magic from my quick look at it. Either you could duplicate what they have stripping out irrelevant code like snapshots, coverage, mocks, or find an AST library that finds implementations of a given file/function(s).

Quick surface level lib research: https://github.com/benjamn/ast-types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants