Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEN-126: Migrate hCore from Webpack 4 to Vite #53

Merged
merged 31 commits into from
Dec 14, 2023
Merged

GEN-126: Migrate hCore from Webpack 4 to Vite #53

merged 31 commits into from
Dec 14, 2023

Conversation

judeallred
Copy link
Collaborator

I was having trouble adding in modern libraries, such as oktokit, without hitting lots of babel and webpack-induced hurdles. By switching to vite, we stop using webpack and babel for 'core', instead making use of vite's defaults.

This review is still a draft-- While the site is online, there's some sort of communication problem with the web workers where they won't start/stop simulations (however single steps work fine). Additionally the 'build' step fails with an OOM error on my computer... this might be resolved by a bigger computer, or ideally by a smarter build config.

@judeallred
Copy link
Collaborator Author

I've got the build sorted now, now just need to sort out what's up with the web workers.

@judeallred
Copy link
Collaborator Author

I've pulled in some foundational upgrades that make this significantly easier to work on.

  • We had a ton of typescript compile errors, and weren't even running tsc. That's no more, I fixed (or disabled) them all and brought 'tsc' into the lint check, so the build has to pass before people can push.
  • I updated to modern lint rules, as recommended by Vite. our eslint and prettier config is now at tip.
  • the modern recommended rules are now in place, whereever possible.
  • Turned on prettier caching to save a bunch of time.
  • renamed yarn's 'fmt-check' to 'lint'
  • fixed a bunch of type bugs in typescript.
  • our redux types are all confused; I sequestered them away but they should all be fixed up in order for us to use redux with types.
  • the 'custom build directory' stuff has all been ripped out. we don't need it anymore and can restore it if we care.
  • webpack is mostly gone, but lingers on as a build runner for jest.
  • lots of package.json removals and version upgrades
  • upgraded typescript to the latest minor version beneath its major

@judeallred
Copy link
Collaborator Author

PR todos:

  • fix the vercel deployment
  • further testing on the web workers

@vilkinsons vilkinsons changed the title Convert core from Webpack 4 -> Vite GEN-126: Migrate hCore from Webpack 4 to Vite Nov 30, 2023
- Turns out promise.resolve().then() doesn't actually accomplish the equivalent of a setImmediate.  I was mislead by something online, oh well...  setTimeout works.
- The main simulation worker was breaking when you used the 'run' functionality because the thread would lock. A setTimeout (and previously, setImmediate) was needed to allow other events in the thread.
@judeallred
Copy link
Collaborator Author

Promoted out of draft mode-- I've got core back to its functionality, but now without a huge amount of aged cruft.

I haven't looked at the Vercel build yet. @CiaranMn if you could take a glance at that I'd be appreciative.

This file was ignored and persisted across branches, thus causing fresh checkouts to break (as Vite no longer generates this) but existing checkouts to act "normal".  I've removed the files and its special cases entirely, and added its contents into types.ts.

This now repairs the build. Next up is pruning out the unused generated items and only keeping the relevant ones.
@judeallred
Copy link
Collaborator Author

Vercel should be fixed. Will return to this and give myself a self-review, then it can merge.

Copy link
Collaborator Author

@judeallred judeallred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed self-review, PR is ready to merge

@vilkinsons vilkinsons self-requested a review December 14, 2023 22:16
@vilkinsons
Copy link
Member

🎉 Awesome work, thanks Jude!

@vilkinsons vilkinsons added this pull request to the merge queue Dec 14, 2023
Merged via the queue into main with commit efe1801 Dec 14, 2023
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants