-
Notifications
You must be signed in to change notification settings - Fork 82
[objective_c] Collections support for NS[Mutable]Dictionary and NS[Mutable]Set
#2234
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
Conversation
PR Health
Breaking changes
|
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| objective_c | Breaking | 7.1.0 | 7.2.0-wip | 8.0.0 Got "7.2.0-wip" expected >= "8.0.0" (breaking changes) |
This check can be disabled by tagging the PR with skip-breaking-check.
Changelog Entry ✔️
| Package | Changed Files |
|---|
Changes to files need to be accounted for in their respective changelogs.
API leaks ⚠️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
| Package | Leaked API symbols |
|---|---|
| objective_c | _Version |
This check can be disabled by tagging the PR with skip-leaking-check.
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
| Files |
|---|
| no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/jni/lib/src/third_party/generated_bindings.dart |
| pkgs/objective_c/lib/src/ns_input_stream.dart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it turns out if a class extends
Baseand mixes inMixin, the methods fromMixinwill override any matching methods fromBase. This means some of the methods onNSMutableArraywere falling back to the slow versions onListMixin, rather than using the fast versions fromNSArray. So I had to duplicate some methods. Same logic applies toNSMutableDictionaryandNSMutableSet.
Why do we have to re-mix-in(!) the ListMixin and others if they're already available in the class hierarchy? We can have NSArray with ListMixin and NSMutableArray extends NSArray and everything should be fine. There is no need to duplicate methods this way. NSMutableArray only overrides the existing modifying methods inherited from its superclass NSArray.
Oh yeah, I can just remove the mixin from the mutable versions. Why didn't I think of that 😅 It's a little more complicated for |
NSDictionaryandNSMutableDictionarynow mix inMapBase, andNSSetandNSMutableSetmix inSetBase. The immutable variants just throw if the mutating methods are called.The implementation is mostly straightforward. To make iteration easier, I also changed
NSEnumeratorto implementIterator. That way, we can use the existing[key/object]Enumerator()methods to iterate the collections without having to add another layer of wrapping.The one quirk is that
NS[Mutable]Dictionary's keys must implement theNSCopyingprotocol, which means that on the Dart side we have a bunch ofNSCopyingobjects in the API, instead of pureObjCObjectBase. But the ObjC API isn't very type safe, so there are still a bunch of places where it usesObjCObjectBasefor the keys. So I had to do somecastFromcalls, and add a_NSDictionaryKeyIteratorthat wraps theNSEnumeratorand casts the elements as it's iterating.Also, it turns out if a class extends
Baseand mixes inMixin, the methods fromMixinwill override any matching methods fromBase. This means some of the methods onNSMutableArraywere falling back to the slow versions onListMixin, rather than using the fast versions fromNSArray. So I had to duplicate some methods. Same logic applies toNSMutableDictionaryandNSMutableSet.Fixes #1444
Part of #1443