supabase / supabase-flutter

Flutter integration for Supabase. This package makes it simple for developers to build secure and scalable products.
https://supabase.com/
MIT License
656 stars 154 forks source link

fix(postgrest): Update parameter type of `match()` filter so that null values cannot be passed. #919

Closed dshukertjr closed 1 month ago

dshukertjr commented 1 month ago

What kind of change does this PR introduce?

Fixes https://github.com/supabase/supabase-flutter/issues/917

What is the current behavior?

Currently, users can pass null to match() filter.

What is the new behavior?

Users will not be able to pass null to match().

Vinzent03 commented 1 month ago

This is technically a breaking change. A map may be of type Map<String,dynamic>, which is not assignable to Map<String, Object>. For example, if you type Map a = {"a": 4,"b":"String"} it's not assignable.

dshukertjr commented 1 month ago

I think we can still consider it a fix for a type that we didn't get right in the first place.

Vinzent03 commented 1 month ago

But the type of a map may be easily Map<dynamic,dynamic> despite having no null entries. So a working code might break now. I would prefer to note it in the documentation and use an assert over the actual values instead.

dshukertjr commented 1 month ago

Fair enough 👍

bdlukaa commented 1 month ago

I like the first proposed solution better. It gives the developers type-safety, causing less hassle. It might be good to have this shipped in the next major release.

dshukertjr commented 1 month ago

I like the first proposed solution better. It gives the developers type-safety, causing less hassle.

I like it too. I think sometimes we try to be a bit too picky about what defines a breaking change. I feel like the world does revolve around these minor "technically breaking changes" being shipped as a bug fix all the time. Flutter itself ships "breaking changes" without bumping the major versions all the time, and as long as it's under moderation, I don't think we should hesitate to ship these changes.

I'd say we just ship it with the original change.