(fix/refactor): rewrite some overbroad try/catches

[agilgur5]

• so there's less silent failures that occur but don't error

• replace overbroad try/catch in getProjectPath with just an
  fs.pathExists

• replace overbroad try/catch in cleanDistFolder with just an fs.remove

  • fs.remove is like rimraf and rm -rf in that it won't error if the
    file/dir doesn't exist
    • if it does error, it's probably because it failed to remove the
      dir, and that should error, because it's potentially a problem,
      especially if you're publishing right after

• rewrite moveTypes() so it doesn't have an overbroad try/catch

  • use fs.pathExists first and early return if it doesn't exist
  • only check for known errors with fs.copy, and rethrow others
  • this way if copy or remove actually fail, they will actually error
    • before they would silently fail, which could similarly be pretty
      bad if one were to publish right after a silent failure

Follow-up to #499 around silent failures

[agilgur5]

Welp, error is in the moveTypes() refactor and that's the one I added last and the only one I wasn't confident in bc fs.copy's error conditions aren't well documented 😅

[agilgur5]

Ok fixed that to only catch some known errors that always occur for some reason (idk why), but moveTypes is to be removed/deprecated in #488 anyway, so maybe I should just leave that out of here as it's problematic anyway...

[swyxio]

these CI failures are annoying :(

[agilgur5]

yea, it really hampers the usability 😞esp since GH Actions don't have a restart button 🙃

So I was gonna say that at this point it's so frequent I feel like we should try lots like #403, but that error actually hasn't happened in a bit and this one here I've gotten locally too... Now that I've seen the errors with fs.copy here I think that's probably the root cause. And notably #468 and #488 haven't run into this error yet (might be coincidence though). Will have to look into some alternatives for fs.copy

[swyxio]

this restart button?

but i also dont like how unpredictable it is :(

[agilgur5]

huh guess they added that at some point, but seems like it's only visible at certain permission levels, bc I don't see that:

[agilgur5]

Actually your UI looks a bit different in general 🤔 My own repos don't quite look that way maybe an A/B test?

[agilgur5]

@sw-yx so I fixed the failing CI issues in #504 (which is a more standalone, non-breaking version of some of the changes in #488 ) and confirmed by running 40+ CI runs.
Also that confirms that this PR passes CI.

[jaredpalmer]

Shit. I didn't realize which was suposed to go first. Sorry about that

[agilgur5]

Yeaaa... I try to split up commits so that the diffs are actually readable and that the commit messages are informative. When squashed, that information/history is lost, which is one reason why I split up some PRs so they could be merged in-order 😕 (merge or even better, rebase, is OK, but TSDX mostly does squashes)
The other main reason is that the purpose of the PRs are different, particularly in this case (a small refactor here vs. a major change/deprecation there), which makes the separation of diffs and messages even more important.

[agilgur5]

So I think the reason why fs.copy might have been erroring here is because it's run multiple times, once per Rollup run (default is 3: esm, cjs dev, and cjs prod). And those are run in parallel with asyncro, so it's copying over itself and removing over itself in parallel (unless the I/O is in serial??).

In #367, I wrote a commit that ensures declarations are only output once (partially because it caused some errors with multi-entry, but I think that was before I code-split them properly, so it only caused errors with multi-"bundle") and makes moveTypes() only called once.
That might resolve why these errors are being thrown. Though this is still deprecated as of #504, some might still use it and the optimization is still a good idea. If we're able to run rpts2 once instead of multiple times as well (if this can be a global plugin while the rest are per-output plugins) then that would have a similar effect and be a much more significant perf boost.

[agilgur5]

So yupppp, my hunch was right: the reason fs.copy was erroring was because of those parallel asyncro build runs. #691 extracts and improves on the commit in #367 that makes moveTypes() only called once.

The try-catch that was narrowed in moveTypes() in this commit/PR has been removed entirely in #691 because the errors no longer occur.

I thought something was amiss when I was looking at those errors (see also #500 (comment)), I even checked out the fs.copy source-code for fs-extra and couldn't find anything that seemed amiss there either, but I just couldn't tell why we were getting errors back then. Race conditions errors are hard to understand!

So yea, #691 puts a lid on this CI errors saga with a root cause fix.
And ironically #367 had that fix well before, before even the CI errors started happening so frequently (due to increased number test fixtures) -- it was actually the first place I started seeing these errors as I wrote in #367 (comment).