Skip to content

Use shared deep-freeze helper for immutable @AckType wrappers#86

Open
leoafarias wants to merge 6 commits intomainfrom
leoafarias/immutable-acktype
Open

Use shared deep-freeze helper for immutable @AckType wrappers#86
leoafarias wants to merge 6 commits intomainfrom
leoafarias/immutable-acktype

Conversation

@leoafarias
Copy link
Collaborator

@leoafarias leoafarias commented Mar 3, 2026

This PR makes @AckType object wrappers immutable and preserves nested immutability at runtime.
It moves deep-freeze logic into shared APIs in package:ack/ack.dart (ackDeepFreeze and ackDeepFreezeObjectMap) and updates code generation to call the shared helper, including aliased Ack imports, instead of emitting per-file freeze helpers.
It also adds Set deep-cloning support to cloneDefault and adds coverage for Set cloning behavior.
Integration and runtime tests were updated to cover immutable map behavior, nested raw-Map mutation blocking, and generated output expectations.
Validation run: dart analyze and dart test in packages/ack, packages/ack_generator, and example, plus build_runner regeneration in example.

@docs-page
Copy link

docs-page bot commented Mar 3, 2026

To view this pull requests documentation preview, visit the following URL:

docs.page/btwld/ack~86

Documentation is deployed and generated using docs.page.

@leoafarias leoafarias requested a review from Copilot March 3, 2026 21:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes generated @AckType object wrappers deeply immutable at runtime by introducing shared deep-freeze helpers in package:ack/ack.dart and updating the generator (and associated tests/examples) to use those helpers instead of emitting per-file freeze utilities.

Changes:

  • Added shared deep-freeze APIs (ackDeepFreeze, ackDeepFreezeObjectMap) to ack and exported them from ack.dart.
  • Updated ack_generator to deep-freeze validated object maps in generated parse/safeParse factories and to return unmodifiable maps from generated args getters.
  • Extended cloneDefault to deep-clone Set values and added tests/fixtures to validate immutability behavior (including nested raw-Map mutation blocking).

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/ack/lib/src/utils/default_utils.dart Adds deep cloning support for Set defaults.
packages/ack/lib/src/utils/deep_freeze_utils.dart Introduces shared deep-freeze helpers used by generated code.
packages/ack/lib/ack.dart Exports the new deep-freeze helper API.
packages/ack/test/default_utils_test.dart Adds coverage for deep cloning of sets and nested contents.
packages/ack_generator/lib/src/builders/type_builder.dart Generator now deep-freezes object wrapper backing maps and updates args/getter casting behavior.
packages/ack_generator/lib/src/validation/model_validator.dart Formatting-only change.
packages/ack_generator/test/integration/ack_type_object_wrapper_immutability_test.dart New integration test validating generated output uses shared deep-freeze helpers.
packages/ack_generator/test/integration/_test.dart, packages/ack_generator/test/.dart Snapshot/expectation updates for new generated output (unmodifiable wrapping / deep-freeze calls).
example/test/verify_implements_works.dart Adds runtime tests ensuring wrapper maps and nested maps reject mutation.
example/lib/*.g.dart Regenerated outputs reflecting deep-freeze usage in factories and unmodifiable args/maps.
packages/ack_json_schema_builder/* Formatting-only test/style updates.
packages/ack_firebase_ai/* Formatting-only test/example updates; no functional conversion changes shown in diff.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… tests

- Remove getter-level Map.unmodifiable() on nested objects since data is
  already deep-frozen at parse time
- Add acyclic-input assumption doc comment to cloneDefault
- Add runtime tests verifying toJson(), args, and nested map mutation throws
- Add aliased Ack import test for deep-freeze helper prefix resolution
- Document toJson() frozen-map behavioral change in CHANGELOG
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1326 to +1329
final conditions = knownKeys.map((k) => "e.key != '$k'").toList();
final filterExpr = conditions.isEmpty
? '_data'
: 'Map.fromEntries(_data.entries.where((e) => ${conditions.join(' && ')}))';
? 'Map<String, Object?>.unmodifiable(_data)'
: 'Map<String, Object?>.unmodifiable(Map.fromEntries(_data.entries.where((e) => ${conditions.join(' && ')})))';
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_buildArgsGetter wraps maps with Map<String, Object?>.unmodifiable(...). In Dart this constructor creates a new unmodifiable copy, so when conditions.isEmpty it will copy _data on every access, and when filtering it will copy twice (Map.fromEntries(...) then Map.unmodifiable(...)). This undermines the stated performance goal and can be costly for large objects. Consider returning _data directly when it’s already deep-frozen, or using an unmodifiable view (e.g., UnmodifiableMapView) to avoid extra copying while still preventing mutation.

Copilot uses AI. Check for mistakes.
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.

2 participants