Typing Updates

[lishaduck]

This doesn't touch implementations, thus the branch name.

Depends on #165

Here's the diff as GH doesn't play well with stacks from forks: https://github.com/lishaduck/node-elm-review/compare/jsdocs...pure-typings

[socket-security[bot]]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@types/fs-extra@9.0.13	None	0	27.9 kB	types
npm/@types/prompts@2.4.9	None	+1	17.9 kB	types
npm/@types/which@2.0.2	None	0	6.06 kB	types
npm/@types/wrap-ansi@8.0.2	None	0	4.27 kB	types

🚮 Removed packages: npm/npm-run-all@4.1.5

View full report↗︎

[lishaduck]

Ah, silly me. I pushed the fix to my next branch. I'll cherry-pick it soonish, and this should be good to go.

[jfmengels]

Alright. Note that I just merged a PR by someone else (https://github.com/jfmengels/node-elm-review/pull/162, the final commit in main being 7ba6bd7ca1af522d21bebe89a66c44ed7d79f211), which may conflict with this branch. If I'm not mistaken, it's only the rename from ruleType to _ruleType that need to be skipped.

[lishaduck]

Alright. Note that I just merged a PR by someone else (#162, the final commit in main being 7ba6bd7), which may conflict with this branch. If I'm not mistaken, it's only the rename from ruleType to _ruleType that need to be skipped.

Thanks for letting me know!

[jfmengels]

CI is failing:

Run turbo run test • Running test • Remote caching disabled x root task //#test (turbo testing) looks like it invokes turbo and might | cause a loop

I think I should have updated CI to run either npm test or turbo testing instead of turbo testing. Does that sound correct to you?

[lishaduck]

CI is failing:

Run turbo run test • Running test • Remote caching disabled x root task //#test (turbo testing) looks like it invokes turbo and might | cause a loop

I think I should have updated CI to run either npm test or turbo testing instead of turbo testing. Does that sound correct to you?

Yeah. I fixed it :)

[lishaduck]

Yeah. I fixed it :)

I take that back. What's up with haste-map? It looks like jest is failing now.

[jfmengels]

Could it be that turbo / CI is caching results from previous runs and that that causes problems?

[lishaduck]

Could it be that turbo / CI is caching results from previous runs and that that causes problems?

I don't think so:

cache bypass, force executing 2d4e737178c4d703

So it's not using the cache. Does jest need to run before test-run? I did parallelize it, which might be the issue.

[lishaduck]

Could it be that turbo / CI is caching results from previous runs and that that causes problems?

I don't think so:

cache bypass, force executing 2d4e737178c4d703

So it's not using the cache. Does jest need to run before test-run? I did parallelize it, which might be the issue.

Hmmm. Locally, test-run works in parallel, which is good. I must've just corrupted it by quitting npm at a bad time. jest is crashing in elm-tooling.test. I'll look into it. After all, this shouldn't change impls. That's the point of this branch.

[lishaduck]

Oh, could it be --detectOpenHandles? I added that to catch some regressions I made, maybe there were some other ones?

Could it be that turbo / CI is caching results from previous runs and that that causes problems?

I don't think so:

cache bypass, force executing 2d4e737178c4d703

So it's not using the cache. Does jest need to run before test-run? I did parallelize it, which might be the issue.

Also, it's reproducible with just npm run jest. Does jest redirect output? console.log at the top of elm-tooling.test.js never logs. It does 😞

[lishaduck]

I figured it out. Somehow, elm-review is parsing jest flags as it's own, but then the log gets overriden by jest so you can't read it. It's probably a bug, but it can go into #156 for now.

[jfmengels]

Thank you for looking into all of this! :pray: