chore: update deno_graph and deno_doc

[kitsonk]

@bartlomieju two test cases are failing, related to compat support and have no idea how the updates broke them:

• integration::compat::import_esm_from_cjs
• integration::compat::require_in_repl

[bartlomieju]

Running these test locally...

[bartlomieju]

@kitsonk this is caused by https://github.com/denoland/deno_std/blob/4e01d764eb86caef2e15c42585f323610903802c/node/stream/consumers.js#L5-L9

deno_graph started understanding these comments and it seems they are broken links. I will fix it in deno_std.

I suggest disabling the tests temporarily before we fix and release new version.

[kitsonk]

Oh yeah, they were just always "broken" in the sense it would just be an any type. Now they actually work and follow standard Deno resolution (unless there is a resolver).

[bartlomieju]

That's good, another small bug caught :) opened denoland/std#1740 to fix it

[kitsonk]

Thinking about this though, I think I need to change deno_graph, dependencies like this shouldn't be a hard fail for the graph, our tsc integration handles these missing fine and we should prevent code from running if a type only dependency is missing. Doing so would be a big change in behaviour, especially if you tried to use code that has just JSDoc type imports that can't be resolved.

[kitsonk]

Hmmm... I have gone back to deno_graph and it is actually behaving like it is expected, in that graph.valid() is "ok" but graph.valid_types_only() errors, as would be expected, which is due to the whole GraphData thing. So I need to ferret out how it is going wrong, and only raises more concerns I have about the whole GraphData abstraction. 😖

[nayeemrmn]

dependencies like this shouldn't be a hard fail for the graph, our tsc integration handles these missing fine and we should prevent code from running if a type only dependency is missing

We used to call graph.valid() which checks runtime deps only, and additionally call graph.valid_types_only() only if type checking is enabled. With the current graph_data.check(), you can toggle that with follow_type_only. It is similarly set based on --no-check status. The abstraction is better. The same problem would have been hit with the old API, it's just now we have newly acknowledged type only deps from JSDoc, and checking those (since type checking is on) isn't working out with some existing code ¯\_(ツ)_/¯

[nayeemrmn]

Also based on #11362, it sounds like we only want to visit JSDoc dependencies for a JS module when either --checkJs is enabled or // @ts-check is written at the top? If that's what tsc does we should do the same.

[kitsonk]

@nayeemrmn yes, basically I am folding that into the logic, but that ended up not being the root cause... the loader appears to actually not be handling missing modules "properly", which is causing a problem.

[kitsonk]

Ok, yeah, it looks like the loader wasn't treating 404's like it was treating files missing locally, causing a difference in behaviour. That is corrected now, and now I am at the point of figuring out the graph valid stuff.

[kitsonk]

I have confirmed this patch fixes #13156.

[kitsonk]

M.O.A.

[bartlomieju]

LGTM