4Catalyzer / found

Extensible route-based routing for React applications
https://4catalyzer.github.io/found/
MIT License
794 stars 55 forks source link

feat: switch build to TS + all tests :tada: #1038

Open golota60 opened 1 year ago

golota60 commented 1 year ago

above and beyonce

things done:

jquense commented 1 year ago

Damn man this awesome.

golota60 commented 1 year ago

i've taken a better glance at build differences - all seem to be related to code-splitting, here they are(left is old right is new)

  1. package.json slight differences(non-issue imo) image
  2. minifying differences(non-issue imo) image
  3. TS declaration file changes - declaration file changes. they just differ cause they're not static declaration files, but are generated by TS. We didn't remove any types, they are just not so neatly packed together as they were previously, they are just a little bit more over the place(will try to fix that in the coming days by moving stuff from typeUtils.ts to files themselves, instead of a one big file) image
  4. server.tsx missing(this just seems random, think it happened cause stuff wasn't explicitly exported - fixed that in newest commit).

So yeah, i think merging this probably won't introduce anything wrong, but since it's a whole switch from JS->TS i expect that some slight issues might come up i've used meld to compare the changes.

Overall I'd say this is ready to be merged(like I said, I expect issues to be small to nonexistent with this), although I'd understand if we want to turn this into a feature branch or something. I'd say we should face it head on and merge this and then deal with any issues while releasing :smile: but that's just me