search

dbos-inc / dbos-transact

The TypeScript framework for backends that scale
https://docs.dbos.dev
MIT License
291 stars 20 forks source link

revert wrong type definitions #468

Closed demetris-manikas closed 1 month ago

demetris-manikas commented 1 month ago

This PR reverts wrong type definitions introduced by #448.

Turns out that ...args: unknown[] is only a problem when used in interface and type definitions While in function definitions is fine.

function func (a: string, ...args: unknown[]) {}
func('asdas', 2, 'adasd', 124)

does not give an error.

Given that I reverted very little of #448

Although I noticed that in user_database.ts there exist definitions like the ones I reverted. https://github.com/dbos-inc/dbos-transact/blob/e554e3c4f493117ed6279f41269530a1fdbbb51b/src/user_database.ts#L13 https://github.com/dbos-inc/dbos-transact/blob/e554e3c4f493117ed6279f41269530a1fdbbb51b/src/user_database.ts#L17 https://github.com/dbos-inc/dbos-transact/blob/e554e3c4f493117ed6279f41269530a1fdbbb51b/src/user_database.ts#L19

If you wish I could change them in another PR

kraftp commented 1 month ago

Hey, unfortunately we have to do a full revert of https://github.com/dbos-inc/dbos-transact/pull/448 (https://github.com/dbos-inc/dbos-transact/pull/469) because it caused breakages in our demo apps and staging environment (multiple breaking changes, not just this). However, we're happy to work with you to re-introduce the non-breaking changes (we think they're good ones!) but more gradually and with more careful testing.

demetris-manikas commented 1 month ago

Can you please provide me with a way to test my changes properly as it seems that running the tests and linting is not enough. I spent quite some to time and I would rather not go back and forth in the future as well.

kraftp commented 1 month ago

Yeah, sorry about this. We test them using the demo apps here: https://github.com/dbos-inc/dbos-demo-apps The problem is that several different changes caused compilation failures in different demo apps.

demetris-manikas commented 1 month ago

Ohhh I see. So I will just add these changes to my branch of #448 and try again at a later time.

demetris-manikas commented 1 month ago

Closing....