Skip to content
This repository was archived by the owner on Jan 6, 2026. It is now read-only.

Implement trackedObject, trackedSet, trackedWeakSet, trackedMap, and trackedWeakMap#1748

Merged
NullVoxPopuli merged 7 commits intomainfrom
nvp/tracked-collections
Jul 27, 2025
Merged

Implement trackedObject, trackedSet, trackedWeakSet, trackedMap, and trackedWeakMap#1748
NullVoxPopuli merged 7 commits intomainfrom
nvp/tracked-collections

Conversation

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli commented Apr 30, 2025

RFC: emberjs/rfcs#1068

Followup to: #1713

Changes from the tracked-built-ins implementation (as described by the RFC)

  • use tags directly, don't use cache / tracked-storage apis (these don't exist anyway)
  • defaults to Object.is for the equals function
    • this means that equality / dirty checking is value-based by default, rather than every assignment causing a dirty
  • has overridable equals function
    • to get tracked-built-ins behavior back, you set equals: () => false
      example:
      class TrackedArray {
        constructor(input) {
          return trackedArray(input, { equals: () => false });
        }
      }
  • description is currently unused, but @wycats has plans -- description is meant for debugger hints

Progress:

  • trackedWeakSet
  • trackedWeakMap
  • trackedSet
  • trackedMap
  • trackedObject

Tasks for self-review

  • all collections have some expectType tests
  • tests for specifying equals

I found an issue with reactivity in the array implementation and I'm investigating that in a separate PR here: #1761

these other collections (for now) are more straight-forward

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2025

This PRmain
Dev
600K └─┬ .
169K   ├── runtime
160K   ├── syntax
100K   ├── compiler
 58K   ├── opcode-compiler
 36K   ├── validator
 27K   ├── manager
 11K   ├── program
8.9K   ├── reference
7.2K   ├── destroyable
6.3K   ├── util
4.3K   ├── node
3.4K   ├── global-context
2.5K   ├── wire-format
1.0K   ├── vm
969B   ├── encoder
844B   ├── vm-babel-plugins
606B   └── owner
588K └─┬ .
169K   ├── runtime
160K   ├── syntax
100K   ├── compiler
 58K   ├── opcode-compiler
 27K   ├── manager
 24K   ├── validator
 11K   ├── program
8.9K   ├── reference
7.2K   ├── destroyable
6.3K   ├── util
4.3K   ├── node
3.4K   ├── global-context
2.5K   ├── wire-format
1.0K   ├── vm
969B   ├── encoder
844B   ├── vm-babel-plugins
606B   └── owner
Prod
236K └─┬ .
 70K   ├── syntax
 63K   ├── runtime
 48K   ├── compiler
 18K   ├── opcode-compiler
9.8K   ├── validator
7.9K   ├── manager
4.8K   ├── program
3.6K   ├── reference
2.4K   ├── util
2.1K   ├── node
1.6K   ├── wire-format
1.5K   ├── destroyable
737B   ├── vm
594B   ├── global-context
516B   ├── encoder
469B   ├── vm-babel-plugins
155B   └── owner
231K └─┬ .
 70K   ├── syntax
 63K   ├── runtime
 48K   ├── compiler
 18K   ├── opcode-compiler
7.9K   ├── manager
5.1K   ├── validator
4.8K   ├── program
3.6K   ├── reference
2.4K   ├── util
2.1K   ├── node
1.6K   ├── wire-format
1.5K   ├── destroyable
737B   ├── vm
594B   ├── global-context
516B   ├── encoder
469B   ├── vm-babel-plugins
155B   └── owner

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/tracked-collections branch from 9e94d43 to f2f862f Compare July 18, 2025 22:15
Comment thread packages/@glimmer-workspace/integration-tests/lib/render-test.ts Outdated
Comment on lines +76 to +91
@test
'get/set existing value'() {
this.assertReactivity(
class extends Component {
map = trackedMap([['foo', 456]]);

get value() {
return this.map.get('foo');
}

update() {
this.map.set('foo', 123);
}
}
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it worth adding a test asserting updating to the same value doesn't cause an update, for the default equals?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do you mean something different from the first test in the file?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I imagined an extra test case at the top which this idea mirrored. nvm this

Comment thread packages/@glimmer-workspace/integration-tests/test/collections/weak-set-test.ts Outdated
Comment thread packages/@glimmer-workspace/integration-tests/test/collections/weak-set-test.ts Outdated
Comment thread packages/@glimmer/validator/lib/collections/map.ts
Comment thread packages/@glimmer/validator/lib/collections/map.ts
@@ -0,0 +1,125 @@
import type { ReactiveOptions } from './types';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❓ just curious if there are existing thoughts on a fully-managed object, where
nested objects/values would be tracked

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how do you mean?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thinking about something that proxies the whole object shape, to handle this type of case

const var = trackedObject({ foo: 123, bar: { zig: 'zag' } });
var.bar.zig = '...'; // reading var.bar is tracked but the write to zig isn't

const bar = var.bar; // read is tracked
bar.zig = '...'; // subsequent reads from here aren't, and update isn't

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You may be interested in this rfc:

emberjs/rfcs#1079

Comment thread packages/@glimmer/validator/lib/collections/set.ts
Comment thread packages/@glimmer/validator/lib/collections/set.ts Outdated
NullVoxPopuli and others added 6 commits July 25, 2025 15:16
Co-authored-by: Chandler Prall <cprall@salesforce.com>
…/weak-set-test.ts

Co-authored-by: Chandler Prall <cprall@salesforce.com>
…/weak-set-test.ts

Co-authored-by: Chandler Prall <cprall@salesforce.com>
class extends Component {
collection = trackedObject({ foo: 123 }, { equals: () => false });
update() {
this.collection.foo = 789;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

commenting this line out has no effect, looks like the test approach isn't correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Perhaps, ya. These were copied directly from tracked-built-ins -- i didn't put any thought in to them.

I would personally test reactivity a bit differently.

But i don't want to do that until we merge the ember and glimmer-vm monorepos

Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@NullVoxPopuli NullVoxPopuli merged commit 765ddb5 into main Jul 27, 2025
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/tracked-collections branch July 27, 2025 14:03
@github-actions github-actions Bot mentioned this pull request Jul 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants