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

Config option for disabling shield errors on obsolete fields #81

Open
samuba opened this issue Aug 26, 2023 · 6 comments
Open

Config option for disabling shield errors on obsolete fields #81

samuba opened this issue Aug 26, 2023 · 6 comments
Labels
enhancement ✨ New feature or request low-prio 🐌

Comments

@samuba
Copy link
Contributor

samuba commented Aug 26, 2023

Problem

Would solve #78.
The shields for telefunc's will throw validation errors when request parameters have fields defined that are not in the type definition. What makes this behaviour problematic is the fact that typescript is structurally typed, meaning it won't help you to find these bugs in the IDE. (if there is at least 1 level of indirection) See Example:

interface User { name: string }

function myTelefunc(user: User) { 
  // implementation
}

// typescript shows error: Object literal may only specify known properties, and 'bar' does not exist in type 'User'
// shield throws validation error for this
myTelefunc({ name: "foo", bar: 1})

// typescript will not show any error for this
// shield throws validation error for this
const userObj = { name: "foo", bar: 1}
myTelefunc(userObj)

See TS playground.

Solution

I propose a new config option which makes the shields not throw validation errors for objects containing fields which are not part of the type definition. This would actually align the shields type validation behaviour with typescripts validation behaviour.

@brillout
Copy link
Owner

Is there a TypeScript config to make this invalid?

const userObj = { name: "foo", bar: 1}
myTelefunc(userObj)

I wonder what TypeScript's rationale is for differentiating this from myTelefunc({ name: "foo", bar: 1}).

As for Telefunc, how can Telefunc tell the difference?

@samuba
Copy link
Contributor Author

samuba commented Aug 26, 2023

I'm pretty sure that there is no config option to make this invalid. This behaviour was a conscious decision in the language design of typescript and is fundamental to a lot of the stuff build upon it. In general typescript types behave more like interfaces from languages like Go.

The big surprise to me was that passing an object literal with an obsolete field is invalid. Maybe their rational was: If you are so explicit in passing it as a literal you most probably want to fully adhere to the type definition. Still feels weird to me though. I consider this an edge case as most of the objects you pass into a function are not literals.
Thats why I think there is not much value for telefunc in finding out if it was passed as literal or not. I'd say, let's optimize for 90% of the cases and make the shields not throw validation errors on objects with obsolete fields, if the config option is set.

@brillout brillout added the enhancement ✨ New feature or request label Aug 26, 2023
@brillout
Copy link
Owner

Makes sense.

The usecase was reading users config from localstorage and then sending it to a telefunc. Some months ago I removed a field from the config and only today found out that apparently some users still have that field in their localstorage, because those requests fail for them.

To be honest, it seems quite an edge case. So I'm leaning towards making this lower priority.

@samuba
Copy link
Contributor Author

samuba commented Aug 27, 2023

I agree that usecase is edge case'y but my proposed solution would help for all other cases also. My other three bugs had no localstorage involved just straight parameter passing and for these I also wished I had the proposed config option.

@samuba
Copy link
Contributor Author

samuba commented Aug 27, 2023

I don't know if I find the time to do this but would you be open for a PR on this?

@brillout
Copy link
Owner

Yes, I can see it to be fairly simple. (If it ends up being complex, keep in mind that I may reject if it's too time consuming for me to review.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request low-prio 🐌
Projects
None yet
Development

No branches or pull requests

2 participants