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.

Sign up for GitHub

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

Jump to bottom

fix: ensure deep mutation ownership widening #11094

Merged
merged 1 commit into from
Apr 8, 2024
Merged

fix: ensure deep mutation ownership widening #11094

merged 1 commit into from
Apr 8, 2024

Conversation

dummdidumm
Copy link
Member

Previously, ownership widening/removal was only done if the immediate object that was encountered was state. This isn't always the case. It also didn't take into account classes with state on it (which turn into getters). This change takes both these cases into account and now always traverses the given object deeply.
fixes #11060

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

@dummdidumm
fix: ensure deep mutation ownership widening
d849558 
Previously, ownership widening/removal was only done if the immediate object that was encountered was state. This isn't always the case. It also didn't take into account classes with state on it (which turn into getters). This change takes both these cases into account and now always traverses the given object deeply.
fixes #11060
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Apr 8, 2024

🦋 Changeset detected

Latest commit: d849558

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dummdidumm dummdidumm merged commit 8578857 into main Apr 8, 2024
8 checks passed
@dummdidumm dummdidumm deleted the deep-ownership-widening branch April 8, 2024 11:57
@github-actions github-actions bot mentioned this pull request Apr 8, 2024
Merged
@michael
Copy link

michael commented Apr 8, 2024

Whooo! Thanks for taking care of this.

Rich-Harris added a commit that referenced this pull request Apr 11, 2024
@Rich-Harris
Revert "fix: ensure deep mutation ownership widening (#11094)"
7810d1f 
This reverts commit 8578857.
@Rich-Harris Rich-Harris mentioned this pull request Apr 11, 2024
Closed
5 tasks
Rich-Harris added a commit that referenced this pull request Apr 12, 2024
@Rich-Harris
Revert "fix: ensure deep mutation ownership widening (#11094)"
f068340 
This reverts commit 8578857.
@Rich-Harris Rich-Harris mentioned this pull request Apr 12, 2024
Merged
trueadm pushed a commit that referenced this pull request Apr 13, 2024
@Rich-Harris
Revert "fix: ensure deep mutation ownership widening" (#11155)
dc6a839 
* Revert "fix: ensure deep mutation ownership widening (#11094)"

This reverts commit 8578857.

* don't delete the changeset, it's already merged
This was referenced Apr 13, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trueadm trueadm trueadm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression: Silence ownership warning for object class fields
3 participants
@dummdidumm @michael @trueadm