Replace the build system, closes #110, #111
Thanks! Vite definitely looks like an improvement over my original custom system.
Regarding the tests, I fetched this branch and played with a little on my machine, and I'm still a puzzled by the failing tests. My assumption right now is that there's something about the Jest mocks that isn't working. The mocks achieve a couple of basic things: replace the asynchronous localstorage code with synchronous code, and preload the test data for tournaments and players. Since the "Loading..." text is appearing in test snapshots now, it doesn't seem like either of those things is happening.
I'll look at it closer once I get a chance. If the mocks are indeed the problem, I'm partially wondering if it's worth keeping them, although then we'd still have to solve the headache of dealing with asynchronous localstorage while testing.
I think I may have found the root issue, although I haven't fixed anything yet to confirm it. It seems that the current vi.mock works differently than jest.mock that was used previously, which I assume is the problem.
Right now, I'm feeling more inclined to avoid mocks entirely by passing the data and DB implementation explicitly. That would likely mean adding parameters to a lot of components though, which would introduce a bigger diff in the source code (which wouldn't necessarily be a bad thing, once it's complete). We can see how well it works or if there's an easier solution.
It looks like this broke the localforage bindings, so I just pushed a commit to fix that. I'm still working on fixing the tests.
It looks like this broke the localforage bindings, so I just pushed a commit to fix that. I'm still working on fixing the tests.
Yes localforage is kinda a problem because they don't support ES6 natively, once you're done or think I can assist just hit me up 👌
I just pushed another commit that should fix the tests. A few notes for posterity:
- After returning to these LocalForage bindings several years later, I think the original ones were way too ambitious. I trimmed them down significantly.
- The previous mocks are now replaced with just one LocalForage replacement that lives inside the
Dbmodule. The module checks an__IS_TEST__variable to decide which storage implementation to use. - The tests are all now async-friendly.
- Before this implementation, I tried using
vitest-indexeddbandlocalforage-memoryStorageDriverpackages to shim the storage, but neither one seemed to work no matter how I invoked them. (LocalForage always complained a compatible driver wasn't available.) Maybe I was doing something wrong, but either way I think the current implementation is superior due to its simplicity.
I'm going to come back and review it again when I get the time (that may be after the holiday here in the US) but this hopefully should be the start of getting things back on track now.
It looks like GitHub actions failures might be due to this issue: https://github.com/jihchi/vite-plugin-rescript/issues/231
@PascalHonegger Feel free to fix that if you're able to. I'll take a closer look once I have a chance, but that probably won't be this week.