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(reactivity): add warning for array operations on null/undefined v… #12732

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

Conversation

Eraybulut34
Copy link

…alues

Enhance the MutableReactiveHandler to include a development-only warning when attempting to perform operations on null or undefined values in arrays. This change aims to prevent unexpected behavior during reactive state management.

…alues

Enhance the MutableReactiveHandler to include a development-only warning when attempting to perform operations on null or undefined values in arrays. This change aims to prevent unexpected behavior during reactive state management.
!Number.isNaN(Number(key))
) {
const targetValue = target[key as keyof typeof target]
if (targetValue === null || targetValue === undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (targetValue === null || targetValue === undefined) {
if (targetValue == null) {

@edison1105 edison1105 added the need test The PR has missing test cases. label Jan 17, 2025
@edison1105
Copy link
Member

Could you please add a test for this?

@skirtles-code
Copy link
Contributor

I'm unclear what problem this warning is trying to solve.

Code like this triggers the warning:

const msg = reactive([])
msg[0] = 7

That seems like normal usage to me.

@edison1105 edison1105 added the need more info Further information is requested label Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more info Further information is requested need test The PR has missing test cases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants