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

Bugfix: support Array as parent in associateDestroyableChild#1771

Open
ef4 wants to merge 1 commit intomainfrom
array-destroyable-parent
Open

Bugfix: support Array as parent in associateDestroyableChild#1771
ef4 wants to merge 1 commit intomainfrom
array-destroyable-parent

Conversation

@ef4
Copy link
Copy Markdown
Contributor

@ef4 ef4 commented Sep 5, 2025

The included test throws an assertion without the accompanying bugfix.

The issue here is that the DestroyableMeta is trying to be clever about avoiding extra allocations, so it uses the OneOrMany type a lot. But this means it cannot accurately distinguish between the case where your parent is an array vs the case where you have multiple parents.

My fix adds a Symbol to the internally-created array that represents multiple parents so it's distinguishable from a single parent that happens to be an array.

The included test throws an assertion without the accompanying bugfix.

The issue here is that the DestroyableMeta is trying to be clever about avoiding extra allocations, so it uses the OneOrMany type a lot. But this means it cannot accurately distinguish between the case where your parent is an array vs the case where you have multiple parents.

My fix adds a Symbol to the internally-created array that represents multiple parents so it's distinguishable from a single parent that happens to be an array.
@ef4 ef4 added the bug label Sep 5, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 5, 2025

This PRmain
Dev
600K └─┬ .
169K   ├── runtime
160K   ├── syntax
100K   ├── compiler
 58K   ├── opcode-compiler
 36K   ├── validator
 27K   ├── manager
 11K   ├── program
8.9K   ├── reference
7.5K   ├── 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
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
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.6K   ├── destroyable
737B   ├── vm
594B   ├── global-context
516B   ├── encoder
469B   ├── vm-babel-plugins
155B   └── owner
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

@ef4
Copy link
Copy Markdown
Contributor Author

ef4 commented Sep 5, 2025

@NullVoxPopuli thanks, I was wondering how to make the performance CI run.

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

We'll need #1772 first 🙈

Silly deprecation means non-working forever thing 🫠

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

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