Skip to content
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

Additional TrackedArray test to ensure that clearing the array by setting length = 0 is possible. #305

Merged
merged 1 commit into from
May 13, 2022

Conversation

ksrb
Copy link

@ksrb ksrb commented May 12, 2022

Added a minor test case, something I wanted to make sure wasn't breaking, feel free to close if redundant.

@chriskrycho
Copy link
Collaborator

Hey @ksrb, thanks for opening this!

I will take a look at the test, and don't mind adding it in principle, but the way the library works we would have to go out of our way to make this not work. All "set" operations run through the proxy that intercepts access, so we'd have to be special-casing setting length to prevent it from being cleared!

Did you hit something that made you doubt this?

@ksrb
Copy link
Author

ksrb commented May 12, 2022

Did you hit something that made you doubt this?

No it works fine in the project I'm working on just wanted to be sure it's more like I saw a similar test and though it would be nice to have as a form of 'documentation' 😄 .

'Similar test'

  test('Can set length on array directly', (assert) => {
    let arr = new TrackedArray();
    arr.length = 123;
    assert.equal(arr.length, 123);
  });

Copy link
Collaborator

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

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

Doesn't hurt to have it! 😄

@chriskrycho chriskrycho merged commit 1913421 into tracked-tools:master May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants