-
-
Notifications
You must be signed in to change notification settings - Fork 407
Add a skipCopy
option to tracked-collections
#1131
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
base: main
Are you sure you want to change the base?
Changes from all commits
d52c7f9
5bf9ab1
4bda530
327b9bc
46411f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
--- | ||
stage: accepted | ||
start-date: 2025-08-13T14:00:00.000Z | ||
release-date: # In format YYYY-MM-DDT00:00:00.000Z | ||
release-versions: | ||
teams: | ||
- framework | ||
prs: | ||
accepted: # Fill this in with the URL for the Proposal RFC PR | ||
project-link: | ||
suite: | ||
--- | ||
|
||
<!--- | ||
Directions for above: | ||
|
||
stage: Leave as is | ||
start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z | ||
release-date: Leave as is | ||
release-versions: Leave as is | ||
teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies | ||
prs: | ||
accepted: Fill this in with the URL for the Proposal RFC PR | ||
project-link: Leave as is | ||
suite: Leave as is | ||
--> | ||
|
||
<!-- Replace "RFC title" with the title of your RFC --> | ||
|
||
# Add a `skipCopy` option to tracked-collections | ||
|
||
## Summary | ||
|
||
Add an option for all tracked-collections (like `trackedArray`, `trackedMap` and `trackedObject`) so that their constructors don't perform a copy of the passed-in collection. | ||
|
||
## Motivation | ||
|
||
That would allow users who know what they're doing to skip a potentially expensive copy of a big collection. | ||
|
||
## Detailed design | ||
|
||
The classes that implement the tracked-collections have an `options` argument. We can add an option to it to skip copying the collection argument that is passed. | ||
|
||
The naming is of course the biggest issue. One might argue that it's good to have "unsafe" in the name because that really is unsafe - if the original collection is modified and it is not copied - that change will not be reflected in the template which might be confusing for the user. On the other hand, this is somewhat "obvious" so naming it "unsafe" might prevent people from using it and therefore skipping on potential (big) performance gains. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I do believe that as well. I am afraid people might see this as "an option that should not be used, unless extreme situation", and might even use it incorrectly. |
||
|
||
Example names: | ||
|
||
```ts | ||
trackedArray(original, { copy: false }); | ||
trackedArray(original, { copySource: false }); | ||
trackedArray(original, { copyUpstream: false }); | ||
|
||
trackedArray(original, { avoidCopy: true }); | ||
trackedArray(original, { skipCopy: true }); | ||
trackedArray(original, { noCopy: true }); | ||
``` | ||
|
||
There is a [PR](https://github.com/glimmerjs/glimmer-vm/pull/1767) in the Glimmer VM repo for an implementation in `trackedArray`. | ||
|
||
## How we teach this | ||
|
||
We add it to the documentation. And also to the TypeScript types (which is already done in the PR above). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section should contain the prose to teach the feature as well as where in the existing guides the feature should be discussed. It should also point out any places in the guides, tutorial or api docs that will need updating. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure the tracked-collections are described in the guides. They are rather new (at least them being built-in in Glimmer - the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code to build them in just landed on main so they will be in the API guides in a version or two. They do appear in places in the guides, https://guides.emberjs.com/release/in-depth-topics/autotracking-in-depth/ is the tracking-focused page There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding the prose here with use-cases helps explain what is being proposed and why. It also prevents RFCs from becoming "stuck" after implementation when we have not yet created docs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kategengler I added some example text to the RFC. Please let me know if that's what you expect. |
||
|
||
Example text for the documentation: | ||
|
||
By default, calling `trackedArray` (and any other tracked-collection constructor) copies the given argument. That is, the following code: | ||
|
||
```ts | ||
let original = [123]; | ||
let arr = trackedArray(original); | ||
original[0]++; // `arr[0]` is *unchanged* | ||
``` | ||
|
||
Will **not** modify the tracked array. | ||
|
||
That could be expensive when a big array is passed. For these cases, there is a second argument with options that can be passed to `trackedArray`: | ||
|
||
```ts | ||
let original = [123]; | ||
let arr = trackedArray(original, { skipCopy: true }); | ||
``` | ||
|
||
Note that this could be dangerous! If the array is modified via its untracked reference (`original` in the example) - that change won't be handled by the tracking mechanism and templates won't redraw. | ||
|
||
## Drawbacks | ||
|
||
Potential confusion in people why their templates are not modified when the original collection is modified. | ||
|
||
## Alternatives | ||
|
||
I don't think there are. For the best performance, no copy should be made. | ||
|
||
## Unresolved questions | ||
|
||
None. |
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.
Yeah, i was thinking we go even shorter and copy: false
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.
I'm fine with any name. Let me know if I should update the RFC text with some specific name.
Uh oh!
There was an error while loading. Please reload this page.
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.
Examples I could think of:
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.
Thanks, I added these to the RFC text.