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

MapBuilder dont fail in compilation time if generics does not match on constructor #255

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielgomezrico
Copy link

@danielgomezrico danielgomezrico commented Jan 28, 2022

Currently you will have a runtime exception if you try to build a map with the wrong types.

This will allow the developer to know at compile time if what he is intending to do will work or not, avoiding bugs.

@dave26199
Copy link

The idea makes sense, but we can't land it like this: it's a breaking change. Because it's a breaking change, we'll want to make all other similar improvements at the same time. That will need considerable work that I don't have time for right now--sorry.

By the way, it's like this for historical reasons, built_collection is from the Dart 1 days when types didn't give guarantees like they do now. So there was no point to having a stricter API back then.

Thanks. I do hope to fix this at some point--but no ETA I'm afraid.

@danielgomezrico
Copy link
Author

Thanks for answering, I created the PR as a draft and empty because I wasn't sure about the change, and I was playing the unit tests to see if I was able to reproduce my runtime issue there.

I understand that this is a breaking change, and your argument, what should I do then? Can you guide me about which other places must be updated? I can add documentation about it too.

@danielgomezrico
Copy link
Author

@dave26199 ping :)

@davidmorgan
Copy link
Contributor

Still no concrete ETA on a breaking change to built_collection, but I do hope to get to it this year; there is a small pile of issues building up.

@danielgomezrico
Copy link
Author

@davidmorgan hi, friendly reminder

@davidmorgan
Copy link
Contributor

Thanks! I've just started making some breaking changes, so this is now in scope :)

But, it will take a while: there are quite a pile of tweaks like this to work through, and I want to get them all in before doing a release.

Hopefully, some time this year :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants