GeezerVison

This is awesome, I think it could be added to 1.9 or 1.8 ... very, very useful.
I think depth 2 would be enough for the majority. So it could be the default.
Cache the findings of Salsa as a pseudo .d.ts (or a real .d.ts in typings/generated) could be great too.
Another approach could be a simple line command option to generate all .d.ts from node_modules that does not have .d.ts or .ts itself. It could help people to write the .d.ts from packages without having to start from 0.

I've not looked at the code but this looks like a very sensible/useful idea

a few minor comments. other than that looks good to me.

Mohamed Hegazy (@mhegazy) Is this still on for a 2.0 release? Is it too late for a 1.9?

I could really use this getting into the 1.9 release.. pretty please!

a 1.9 release would help us tremendously also

+1 for getting this in the 1.9 release - would be a great help for us.

It would be great if this made it to 1.9

Bill Ticehurst (@billti) could you resolve the conflicts?

When could this PR be merged? I look forward to see #6670 resolved

cc/ egamma

Thanks!

Bill Ticehurst (@billti) is this ready to go? can you update?

I'll try and get to it later tonight or tomorrow... just got a couple of other tasks ahead of it.

They say necessity is the mother of invention... by rethinking how to handle this with the new requirement that a SourceFile might be reused across programs, I think this is actually much simpler now.

Mohamed Hegazy (@mhegazy), can you take a look and let me know if you spot any flaws with this approach?

I don't handle the suppression of emit if the file is located under node_modules, but that is a separate issue that is present already. I can add that and more tests if desired before checking in.

Actually, trying to solve the emit problem I realized where this falls down in the general case.

If module A imports B imports C, and is doesn't add C when processing B because it's too many hops, if I then later directly import B, is will already have processed the file and won't reprocess imports and see that C is now not too far and should be added. 😢 Back to the drawing board...

👍

Can you also add a test for --maxNodeModuleJsDepth

Mohamed Hegazy (@mhegazy) This should be pretty comprehensive now. It tracks if a JavaScript file was loaded from searching node_modules so it knows not to compile it (a minor tweak to this logic could also solve #6964 if desired). It also tracks if a module has some imports skipped, so that if the same module is later processed higher up the import graph it will re-processes the imports.

I added a few project tests to test a) the eliding once the max depth is reached,b) increasing the depth via the config option, and c) the reprocessing of imports if a module is later imported higher up the chain. I used Project tests as with baselines its hard to add files that exist but aren't root files (as the files under node_modules need to be imported but not root files). The project tests also contain useful info about what files were input, resolved, and output.

Let me know if you have any other suggestions for improvements.

p.s. I had to fix a regular expression to get the Project tests working with the .js files (else it was putting a full path from my machine in the error baselines), but that did change a lot of other baselines (for the better).

👍

How about this change?

Note that I'm using a Map for the collection of file names as this seems quicker on large projects with lots of modules from node_modules, than using a array of file paths and scanning this every time to check for existence (and splicing when I need to remove an entry). I could use a simple string[] if that seems cleaner, but this works well currently.

I would have preferred a function that takes a file path and returns a boolean. but this is fine too.

can you mark this as /* @internal */ since we are the only users for it.

Like so?

thanks!

The build is taking forever :-/ I'm sure it'll pass... but waiting as a precaution.

I'll port and merge this to release-2.0 also unless you have any objections.

no objections. thanks!

👏 Bill Ticehurst (@billti) Mohamed Hegazy (@mhegazy)