Fix path splitting issues on windows

[iterion]

It's not safe to assume files are delimited by /. Instead, electron re-exports the node path lib which we can use for dirname, basename, etc.

I found another occurence of this that must be causing some bugs somewhere in routeLoaders. I've fixed it and verified it still works on linux.

I haven't tested this on windows yet, but everything still functions on linux so it's at least notionally better. I'll work on getting a dev env on my windows machine.

[qa-wolf]

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
modeling-app	✅ Ready (Inspect)	Visit Preview	Aug 22, 2024 5:27pm

[jessfraz]

jesus we need a lint for shit like this haha makes me nervous just looking at it

[jessfraz]

@iterion i think you need to run yarn fmt

[iterion]

yes, idgaf about that until i'm done lol, editor formats it in a completely different way so I just disable it and format once i'm done.

[jessfraz]

can we unit test in the same way as the rust tests were, only becaiuse there were lots of regression tests in those unit tests, aka those little shits are load bearing :)

[iterion]

Yeah, I can unit test as-is, I took a lot of stuff out because it seems unnecessary. Like, the old code was not using half of the function because of a bug with how config was passed in a json, we were just getting lucky before that this worked even with unit tests.

[iterion]

If you know any of the specific functionality regressions I could write e2e tests for those too, to make sure I didn't cut out currently important stuff.

[jessfraz]

mostly files w spaces, either url encoded or not were weird

[jessfraz]

idk if applies here

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.83%. Comparing base (682099c) to head (1207c65).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3565      +/-   ##
==========================================
- Coverage   87.90%   87.83%   -0.07%     
==========================================
  Files          67       67              
  Lines       26150    25960     -190     
==========================================
- Hits        22986    22803     -183     
+ Misses       3164     3157       -7     

Flag	Coverage Δ
wasm-lib	87.83% <ø> (-0.07%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jtran]

This doesn't get cleaned up between runs. I really don't like modifying global. It's easy to make mistakes like this. Here, it introduces test order dependence, and that's a best case scenario. I'd much prefer having abstraction functions. But if we really can't get around it, I think we need to move this to a beforeEach() and in an afterEach(), remove it from global. This way, if there's an exception, it still cleans up.

[iterion]

I switched to just injecting path into the function, we can then remove onDesktop as a param as it basically just checks if it's electron, and we know it's electron when window.electron.path is defined.

[iterion]

Ha, that's broken though, not working when I test it. Maybe back to beforeEach for now.

[jtran]

we can then remove onDesktop as a param as it basically just checks if it's electron, and we know it's electron when window.electron.path is defined.

I think this makes sense. Then add a local var like this.

const isDesktop = !!pathlib

[iterion]

False alarm, CI looks pretty good outside of the known flakes. I tested again locally after rebuilding everything and it worked there too.