fix(typegen): avoid possible infinite recursion error #630
Conversation
avallete
force-pushed
the
fix/limit-types-depth-recursion
branch
from
04c89fb to
721b142
Compare
There was a problem hiding this comment.
After attempting many things:
(based on: https://www.esveo.com/en/blog/how-to-workaround-the-max-recursion-depth-in-typescript/)
- Tail recursion
- Batches handling
- Reseting counter to 0
None of them really improved the issue, I've figured out that this error is mainly a regression due to the newly introduced Omit. It'll still happen for complex queries though.
The only "optimization" that had a bit of value were hardcoding some "batches" to reduce the recursion inference. It didn't allowed much deeper recursions though, only improved a bit the inference speed (~10%). Might introduce this in future changes, keeping this one focused on reversing the regression and adding tests cases for it.
| @@ -40,8 +40,8 @@ | |||
| "test": "run-s format:check test:types db:clean db:run test:run db:clean && node test/smoke.cjs && node test/smoke.mjs", | |||
| "test:run": "jest --runInBand --coverage", | |||
| "test:update": "run-s db:clean db:run db:generate-test-types && jest --runInBand --updateSnapshot && run-s db:clean", | |||
| "test:types": "run-s build && tsd --files 'test/**/*.test-d.ts'", | |||
| "test:types:watch": "run-s build && tsd --files 'test/**/*.test-d.ts' --watch", | |||
| "test:types": "run-s build && tsd --files 'test/**/*.test*.ts'", | |||
There was a problem hiding this comment.
Now that we have types and tests merged together, we want to check for all tests files that the types assertions passes.
package.json
Outdated
| "test:types": "run-s build && tsd --files 'test/**/*.test-d.ts'", | ||
| "test:types:watch": "run-s build && tsd --files 'test/**/*.test-d.ts' --watch", | ||
| "test:types": "run-s build && tsd --files 'test/**/*.test*.ts'", | ||
| "test:types:watch": "chokidar 'src/select-query-parser/**/*.ts' 'test/**/*.ts' -c 'npm run test:types'", |
There was a problem hiding this comment.
Previous watcher weren't working. Use chodikar for better DX to check the types errors.
- Add a fixed max depth to recursive types allowing Typescript to not raise possible infinite recursion error too early. - Add test to ensure that the provided fix remove this error from a recursive embeding Related: supabase/supabase-js#1354 supabase/supabase-js#1372 supabase/supabase-js#808
Fixes regression introduced in #627 While fixing the invalid intersection for conflicting keys case the Omit did lead to a huge increase in the recursive type complexity Removing it bring back the corner case, but allow much more longer queries. A test is now in place to ensure minimal coverage over the query complexities we can handle before reaching infinite recursion errors.
avallete
force-pushed
the
fix/limit-types-depth-recursion
branch
from
721b142 to
876014e
Compare
|
🎉 This PR is included in version 1.21.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
OmitRelated:
supabase/supabase-js#1354
supabase/supabase-js#1372
supabase/supabase-js#808