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

fix: correctly copy getters in deepCyclicCopy #15265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Aug 20, 2024

Summary

Fixes #15196

deepCyclicCopyReplaceable just accesses the value unconditionally, but we have tests in deepCyclicCopy that getters are not accessed.

value: deepCyclicCopyReplaceable(
// this accesses the value or getter, depending. We just care about the value anyways, and this allows us to not mess with accessors
// it has the side effect of invoking the getter here though, rather than copying it over
(object as Record<string | symbol, unknown>)[key],
cycles,
),

Not sure if this is the correct approach, but seems fine? Let's see what CI says

Test plan

Unit test added

Copy link

netlify bot commented Aug 20, 2024

Deploy Preview for jestjs ready!

Name Link
🔨 Latest commit 2ecfd4e
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/66c44bacf6a7490008b7f99c
😎 Deploy Preview https://deploy-preview-15265--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@SimenB SimenB changed the title fix: correctly access getters in deepCyclicCopy fix: correctly copy getters in deepCyclicCopy Aug 20, 2024
@SimenB SimenB force-pushed the handle-errors-in-cyclic-copy branch 2 times, most recently from 920f35d to a3694e0 Compare August 20, 2024 07:54
@SimenB SimenB force-pushed the handle-errors-in-cyclic-copy branch from a3694e0 to 2ecfd4e Compare August 20, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: [jest-runner] loses error stack property for failed test when sends a message to jest
1 participant